Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove new keyword in a few files #104438

Merged
merged 17 commits into from
Jun 6, 2022
Merged

Remove new keyword in a few files #104438

merged 17 commits into from
Jun 6, 2022

Conversation

nilsreichardt
Copy link
Contributor

There were files with the old keyword.

we really should find a way to analyze this

Wrote @Hixie in https://github.com/flutter/flutter/pull/69507/files#r519475809

Therefore, I tried to find all old new keywords. I think I fixed them all. But there are still some that I wasn't sure were on purpose there:

  • analyze_snippet_code_test.dart L28, L30
  • known_broken_documentation.dart L30, 62
  • assertions_test.dart L356, L360, L391, L395, L396
  • error_reporting_test.dart L227, L249
  • stack_frame_test.dart L107, L123, L140, L198
  • expression_evaluation_test.dart L160, L166

Fixes #104436

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.

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

@nilsreichardt
Remove new keyword in common.dart
9ba6a41
@nilsreichardt
Remove new keyword in text_style.dart
f010bd1
@nilsreichardt
Remove new keyword in platform_view.dart
324eb54
@nilsreichardt
Remove new keyword in preferred_size.dart
902bd5f
@nilsreichardt
Remove new keyword in silver.dart
03d1fd5
@nilsreichardt
Remove new keyword in colors_test.dart
7b6f238
@nilsreichardt
Remove new keyword in driver_extension.dart
9b1ddb4
@nilsreichardt
Remove new keyword in enum_util.dart
38a6900
@nilsreichardt
Remove new keyword in cupertino_localizations.dart
39f37a0
@nilsreichardt
Remove new keyword in material_localizations.dart
d383291
@nilsreichardt
Remove new keyword in nonconst.dart
dd61ea3
@nilsreichardt
Remove new keyword in basic_project.dart
6f75ada
@nilsreichardt
Remove new keyword in deferred_components_project.dart
24c8196
@nilsreichardt
Remove new keyword in multidex_project.dart
8ec3761
@nilsreichardt
Remove new keyword in project_with_early_error.dart
9878d6a
@nilsreichardt
Remove new keyword in stepping_project.dart
d543fe5
@flutter-dashboard flutter-dashboard bot added a: internationalization Supporting other languages or locales. (aka i18n) a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino 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. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels. labels May 23, 2022
nilsreichardt
nilsreichardt commented May 23, 2022
View reviewed changes
packages/flutter/lib/src/painting/text_style.dart
Comment on lines 16 to +20
const String _kColorForegroundWarning = 'Cannot provide both a color and a foreground\n'
'The color argument is just a shorthand for "foreground: new Paint()..color = color".';
'The color argument is just a shorthand for "foreground: Paint()..color = color".';

const String _kColorBackgroundWarning = 'Cannot provide both a backgroundColor and a background\n'
'The backgroundColor argument is just a shorthand for "background: new Paint()..color = color".';
'The backgroundColor argument is just a shorthand for "background: Paint()..color = color".';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I add tests for this?

@goderbauer
Copy link
Member

Ideally, we'd wrap the code samples in {@tool snippet} sections so they would get analyzed...

goderbauer
goderbauer approved these changes May 25, 2022
View reviewed changes
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

Thanks for the clean-up.

Piinks
Piinks approved these changes May 25, 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.

LGTM!

@Piinks Piinks added the c: tech-debt Technical debt, code quality, testing, etc. label May 25, 2022
@nilsreichardt
Copy link
Contributor Author

What is with the files in the description?

analyze_snippet_code_test.dart L28, L30
known_broken_documentation.dart L30, 62
assertions_test.dart L356, L360, L391, L395, L396
error_reporting_test.dart L227, L249
stack_frame_test.dart L107, L123, L140, L198
expression_evaluation_test.dart L160, L166

Should I open new a PR and you just say if removing the new was not correct?

@Piinks
Copy link
Contributor

Piinks commented May 25, 2022

Those look fine to me, I do not think they need to be changed. Thanks!

@christopherfujino
Copy link
Member

CI is pending on your PR because there were problems upstream on master at the time you branched. If you rebase upstream, CI will re-run and checks should pass. Sorry about the trouble and thanks for contributing!

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Mac_android run_release_test has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot merged commit 64db621 into flutter:master Jun 6, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
ad50401
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
40bb656
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
db68289
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
97fdb96
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
158691f
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
048a99d
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
be05950
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 7, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 7, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
a5b9d24
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@nilsreichardt @camsim99
Remove new keyword in a few files (flutter#104438)
a2da2b9
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
28ff30e
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
64db621 Remove new keyword in a few files (flutter/flutter#104438)
a988df8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goderbauer goderbauer goderbauer approved these changes

@Piinks Piinks Piinks approved these changes

Assignees
No one assigned
Labels
a: internationalization Supporting other languages or locales. (aka i18n) a: tests "flutter test", flutter_test, or one of our tests a: text input Entering text in a text field or keyboard related problems c: tech-debt Technical debt, code quality, testing, etc. f: cupertino flutter/packages/flutter/cupertino 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. team Infra upgrades, team productivity, code health, technical debt. See also team: labels. tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

There is still code with the old new keyword
5 participants
@nilsreichardt @goderbauer @Piinks @christopherfujino @fluttergithubbot