Skip to content

Conversation

@EchoEllet
Copy link
Contributor

@EchoEllet EchoEllet commented May 1, 2024

Use super.key instead of manually passing the Key parameter using super(key: key) in the constructors.

Since if you create a widget the new default will use super.key instead of Key? key : super(key: key) this small change is to maintain the consistency, it has no semantic change

also there are some other places that might need to be updated:

image

this file for example generate l10n project and it has all the dart code as String, it might have tests that validate the output somewhere that I might miss, also there are some other places like the _Segment class where it require ValueKey instead if Key so I didn't update them (even though it's possible)

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels May 1, 2024
@goderbauer goderbauer self-requested a review May 1, 2024 19:51
@goderbauer
Copy link
Member

Thanks for the clean-up. Looks like the analyzer is not happy, though: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8749165137680142961/+/u/run_test.dart_for_analyze_shard_and_subshard_None/stdout

Could you look into that and fix whatever it is complaining about?

Copy link
Contributor

@nate-thegrate nate-thegrate left a comment

Choose a reason for hiding this comment

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

Thanks a bunch for the contribution!

Sorry to make a bunch of suggestions at once, but hopefully we can get all those tests to start passing soon 😄

@EchoEllet
Copy link
Contributor Author

EchoEllet commented May 2, 2024

Thanks a bunch for the contribution!

Sorry to make a bunch of suggestions at once, but hopefully we can get all those tests to start passing soon 😄

Thank you, too, for all your amazing work in this framework, I wasn't planning on contributing to Flutter, but I found one of the widget in flutter source code that still passing it to the super class by manually creating the Key, so I searched using the IDE and found all of this. I will do what you suggested as soon as I can

Copy link
Contributor

@nate-thegrate nate-thegrate left a comment

Choose a reason for hiding this comment

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

Sorry to hear about your slow network connection! Let's see if we can get these tests passing…

@github-actions github-actions bot removed a: text input Entering text in a text field or keyboard related problems f: scrolling Viewports, list views, slivers, etc. labels May 13, 2024
@goderbauer
Copy link
Member

For some context: There is a use_super_parameters lint that is supposed to encourage the use of super. everywhere. Unfortunately, it has a bug and doesn't trigger when the type of the parameter changes (as is the case with all the instances in this PR), see dart-lang/sdk#59226.

Copy link
Contributor

@nate-thegrate nate-thegrate left a comment

Choose a reason for hiding this comment

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

Flutter_LGTM

Thanks again for the fantastic contribution!

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you very much!

@goderbauer goderbauer added the autosubmit Merge PR when tree becomes green via auto submit App label May 13, 2024
@EchoEllet
Copy link
Contributor Author

For some context: There is a use_super_parameters lint that is supposed to encourage the use of super. everywhere. Unfortunately, it has a bug and doesn't trigger when the type of the parameter changes (as is the case with all the instances in this PR), see dart-lang/sdk#59226.

Thank you for the info. I didn't know about it

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 14, 2024
flutter/flutter@1255435...d2da1b2

2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 491f460a25fb to 7bf865774d06 (1 revision) (flutter/flutter#148325)
2024-05-14 engine-flutter-autoroll@skia.org Roll Packages from 1412041 to fd714bd (1 revision) (flutter/flutter#148324)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from c381b852605f to 491f460a25fb (1 revision) (flutter/flutter#148319)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from b4b798d2e706 to c381b852605f (1 revision) (flutter/flutter#148303)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0de6701b537a to b4b798d2e706 (2 revisions) (flutter/flutter#148299)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 764c33c3c773 to 0de6701b537a (2 revisions) (flutter/flutter#148297)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 797eab742925 to 764c33c3c773 (1 revision) (flutter/flutter#148290)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 84687fe0f199 to 797eab742925 (1 revision) (flutter/flutter#148288)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from bee398d95abe to 84687fe0f199 (3 revisions) (flutter/flutter#148282)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from a1cdcb6a6687 to bee398d95abe (2 revisions) (flutter/flutter#148280)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 13a561cb6d5a to a1cdcb6a6687 (1 revision) (flutter/flutter#148276)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7dcbd93e5c1a to 13a561cb6d5a (6 revisions) (flutter/flutter#148274)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from aeff9b174c84 to 7dcbd93e5c1a (1 revision) (flutter/flutter#148266)
2024-05-13 magder@google.com Mark platform_views_scroll_perf_ad_banners__timeline_summary not flaky (flutter/flutter#148263)
2024-05-13 andrewrkolos@gmail.com add more print traces in hot runner workflow (flutter/flutter#148258)
2024-05-13 47866232+chunhtai@users.noreply.github.com Refactors page API (flutter/flutter#137792)
2024-05-13 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.3 to 3.25.5 (flutter/flutter#148262)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 11502404a52a to aeff9b174c84 (2 revisions) (flutter/flutter#148260)
2024-05-13 73608287+ellet0@users.noreply.github.com Use super.key instead of manually passing the Key parameter to the parent class (flutter/flutter#147621)
2024-05-13 dacoharkes@google.com Try fix module test (flutter/flutter#147934)
2024-05-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland Native ios context menu (#143002) (#148238)" (flutter/flutter#148254)
2024-05-13 jmccandless@google.com Reland Native ios context menu (#143002) (flutter/flutter#148238)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 636374fd00ee to 11502404a52a (1 revision) (flutter/flutter#148242)
2024-05-13 engine-flutter-autoroll@skia.org Roll Packages from 6c4482a to 1412041 (16 revisions) (flutter/flutter#148239)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0050bf9a8094 to 636374fd00ee (1 revision) (flutter/flutter#148216)
2024-05-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Native ios context menu (#143002)" (flutter/flutter#148237)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
flutter/flutter@1255435...d2da1b2

2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 491f460a25fb to 7bf865774d06 (1 revision) (flutter/flutter#148325)
2024-05-14 engine-flutter-autoroll@skia.org Roll Packages from 1412041 to fd714bd (1 revision) (flutter/flutter#148324)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from c381b852605f to 491f460a25fb (1 revision) (flutter/flutter#148319)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from b4b798d2e706 to c381b852605f (1 revision) (flutter/flutter#148303)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0de6701b537a to b4b798d2e706 (2 revisions) (flutter/flutter#148299)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 764c33c3c773 to 0de6701b537a (2 revisions) (flutter/flutter#148297)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 797eab742925 to 764c33c3c773 (1 revision) (flutter/flutter#148290)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 84687fe0f199 to 797eab742925 (1 revision) (flutter/flutter#148288)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from bee398d95abe to 84687fe0f199 (3 revisions) (flutter/flutter#148282)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from a1cdcb6a6687 to bee398d95abe (2 revisions) (flutter/flutter#148280)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 13a561cb6d5a to a1cdcb6a6687 (1 revision) (flutter/flutter#148276)
2024-05-14 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7dcbd93e5c1a to 13a561cb6d5a (6 revisions) (flutter/flutter#148274)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from aeff9b174c84 to 7dcbd93e5c1a (1 revision) (flutter/flutter#148266)
2024-05-13 magder@google.com Mark platform_views_scroll_perf_ad_banners__timeline_summary not flaky (flutter/flutter#148263)
2024-05-13 andrewrkolos@gmail.com add more print traces in hot runner workflow (flutter/flutter#148258)
2024-05-13 47866232+chunhtai@users.noreply.github.com Refactors page API (flutter/flutter#137792)
2024-05-13 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.3 to 3.25.5 (flutter/flutter#148262)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 11502404a52a to aeff9b174c84 (2 revisions) (flutter/flutter#148260)
2024-05-13 73608287+ellet0@users.noreply.github.com Use super.key instead of manually passing the Key parameter to the parent class (flutter/flutter#147621)
2024-05-13 dacoharkes@google.com Try fix module test (flutter/flutter#147934)
2024-05-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reland Native ios context menu (#143002) (#148238)" (flutter/flutter#148254)
2024-05-13 jmccandless@google.com Reland Native ios context menu (#143002) (flutter/flutter#148238)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 636374fd00ee to 11502404a52a (1 revision) (flutter/flutter#148242)
2024-05-13 engine-flutter-autoroll@skia.org Roll Packages from 6c4482a to 1412041 (16 revisions) (flutter/flutter#148239)
2024-05-13 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0050bf9a8094 to 636374fd00ee (1 revision) (flutter/flutter#148216)
2024-05-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Native ios context menu (#143002)" (flutter/flutter#148237)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants