[palette_generator] Fix dynamic calls due to operator == #744
Conversation
|
This pull request was opened against a branch other than main. Since Flutter pull requests should not normally be opened against branches other than main, I have changed the base to main. If this was intended, you may modify the base back to master. See the Release Process for information about how other branches get updated. Reviewers: Use caution before merging pull requests to branches other than main, unless this is an intentional hotfix/cherrypick. |
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
1 similar comment
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
What do I need to do to satisfy the publishable requirement? I don't think this needs a version update. so I tried to add "No version change: Minor type fixes - no functional changes." but it didn't seem to be registered by the tool. |
|
uuhhh that one is definitely new. I would still bump the minor version otherwise we won't be able to publish, though I'm not sure how that impacts g3. FYI @Hixie |
|
You need to change the PR after updating the comment, so that it retriggers the test. Easiest way to do that is to add a test, which you will need to do anyway. :-) |
|
Ahh yeah, I guess technically you could test that there is no NoSuchMethodError thrown when comparing with an Object that doesn't implement the comapred members. I believe there would be one before |
This fixes a client-affecting issue; why do you think it doesn't need a version update? In particular, did you read the list of exemptions that was linked from the checklist? |
|
Added tests and bumped the pubspec. This should be able to land now. |
| @@ -1,3 +1,7 @@ | |||
| ## 0.3.3 | |||
|
|
|||
| * Avoid dynamic calls in equality checks. | |||
There was a problem hiding this comment.
Nit: Avoids
(We now have a style guide for CHANGELOG entries.)
Dynamic calls can cause symbols to be retained that otherwise could be tree shaken.
This change checks that 'other' is of the same type in the operator == before referencing its member variables.
Fixes: flutter/flutter#98075