Skip to content

Migrates packages/flutter to from hashValues to Object.hash#96109

Merged
werainkhatri merged 6 commits intoflutter:masterfrom
werainkhatri:migrate-hash
Mar 1, 2022
Merged

Migrates packages/flutter to from hashValues to Object.hash#96109
werainkhatri merged 6 commits intoflutter:masterfrom
werainkhatri:migrate-hash

Conversation

@werainkhatri
Copy link
Member

@werainkhatri werainkhatri commented Jan 4, 2022 •
edited
Loading

Migrates packages/flutter from hashValues and hashList to Object.hash and Object.hashAll. This series of PRs (for all packages) will migrate them before deprecating hashValues and hashList.

#85431 (Doesn't close yet, as other packages need to be migrated)

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Jan 4, 2022
@werainkhatri werainkhatri force-pushed the migrate-hash branch 2 times, most recently from a54505d to 2adb804 Compare January 4, 2022 14:26
werainkhatri
werainkhatri commented Jan 4, 2022
View reviewed changes
bernaferrari
bernaferrari reviewed Jan 4, 2022
View reviewed changes
@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. labels Jan 4, 2022
@werainkhatri werainkhatri mentioned this pull request Jan 4, 2022
Merged
13 tasks
@werainkhatri werainkhatri marked this pull request as ready for review January 6, 2022 19:26
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
Hixie
Hixie reviewed Jan 6, 2022
View reviewed changes
@Hixie
Copy link
Contributor

Hixie commented Jan 6, 2022

it would be great to also add an automatic fix so that people can migrate using dart fix --apply.

@werainkhatri
Copy link
Member Author

werainkhatri commented Jan 6, 2022 •
edited
Loading

it would be great to also add an automatic fix so that people can migrate using dart fix --apply.

That can be easily done for hashValues, but since hashList supported nullable list while Object.hashAll doesn't, how do I handle that?

EDIT: currently, I have just added a 'rename' kind fix, for hashList -> Object.hashAll, which may cause error to the user if the arg is nullable.

@werainkhatri werainkhatri requested a review from Hixie January 7, 2022 00:07
@werainkhatri
Copy link
Member Author

werainkhatri commented Jan 25, 2022

I removed the fix_data entries, as we are aiming for a large-scale dart fix (dart-lang/sdk#48204). Other than the failing checks (which idk why they are failing), this PR is ready.

This was referenced Jan 28, 2022
Closed
Closed
@werainkhatri werainkhatri force-pushed the migrate-hash branch 2 times, most recently from fdbd0e5 to 05423ee Compare February 4, 2022 16:35
@werainkhatri werainkhatri mentioned this pull request Feb 9, 2022
Closed
8 tasks
Piinks
Piinks approved these changes Feb 17, 2022
View reviewed changes
Copy link
Contributor

@Piinks Piinks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM - thank you! It looks like there are some outstanding merge conflicts though.

@skia-gold
Copy link

skia-gold commented Feb 18, 2022

Gold has detected about 38 new digest(s) on patchset 7.
View them at https://flutter-gold.skia.org/cl/github/96109

1 similar comment
@skia-gold
Copy link

skia-gold commented Feb 18, 2022

Gold has detected about 38 new digest(s) on patchset 7.
View them at https://flutter-gold.skia.org/cl/github/96109

@werainkhatri
Copy link
Member Author

werainkhatri commented Feb 18, 2022

@Piinks Fixed the conflicts, can't seem to fix these checks.

@Piinks
Copy link
Contributor

Piinks commented Feb 22, 2022 •
edited
Loading

Did you update with a merge, or a rebase? I usually merge, but it can lead to issues with CI when a PR has gotten a bit old. Rebasing usually resolves it in that case.

@werainkhatri
Copy link
Member Author

werainkhatri commented Feb 23, 2022 •
edited
Loading

I have rebased. I have been rebasing for a long time in this PR itself, and these checks have failed every time.

@werainkhatri werainkhatri merged commit 078deb9 into flutter:master Mar 1, 2022
@werainkhatri werainkhatri deleted the migrate-hash branch March 1, 2022 18:24
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 1, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 1, 2022
@engine-flutter-autoroll
078deb9 Migrates packages/flutter to from hashValues to `Object.has…
c54b005 
…h` (flutter/flutter#96109)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 1, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Mar 1, 2022
@engine-flutter-autoroll
078deb9 Migrates packages/flutter to from hashValues to `Object.has…
1e4503c 
…h` (flutter/flutter#96109)
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Mar 8, 2022
@werainkhatri @clocksmith
Migrates packages/flutter to from hashValues to Object.hash (flut…
72b6ef5 
…ter#96109)

* migrates lib/foundation and lib/widgets

* migrates lib/painting and lib/material

* migrates lib/cupertino, lib/gestures, lib/services, lib/semantics and lib/rendering

* fixes failing tests

* migrates test/

* improves hashing of lists
@irvine5k irvine5k mentioned this pull request Dec 16, 2024
Merged
@praiaBarryTolnas praiaBarryTolnas mentioned this pull request Dec 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bernaferrari bernaferrari bernaferrari left review comments

@Hixie Hixie Hixie left review comments

@bwilkerson bwilkerson bwilkerson left review comments

@Piinks Piinks Piinks approved these changes

Assignees

No one assigned

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: text input Entering text in a text field or keyboard related problems c: contributor-productivity Team-specific productivity, code health, technical debt. c: tech-debt Technical debt, code quality, testing, etc. f: cupertino flutter/packages/flutter/cupertino repository f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@werainkhatri @Hixie @bernaferrari @skia-gold @Piinks @bwilkerson