Skip to content

Enable avoid_redundant_argument_values lint #91409

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
Merged
merged 6 commits into from
Oct 8, 2021
Merged

Enable avoid_redundant_argument_values lint #91409

merged 6 commits into from
Oct 8, 2021

Conversation

Hixie
Copy link
Contributor

@Hixie Hixie commented Oct 7, 2021

No description provided.

@google-cla google-cla bot added the cla: yes label Oct 7, 2021
@flutter-dashboard flutter-dashboard bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: animation Animation APIs 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 d: examples Sample code and demos f: cupertino flutter/packages/flutter/cupertino repository f: focus Focus traversal, gaining or losing focus f: gestures flutter/packages/flutter/gestures repository. f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. f: integration_test The flutter/packages/integration_test plugin c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Oct 7, 2021
@Hixie Hixie force-pushed the default_arguments branch from ea96797 to 11472c8 Compare October 7, 2021 05:05
@Hixie
Copy link
Contributor Author

Hixie commented Oct 7, 2021 •
edited
Loading

Reviewing the commits separately may be easier. There are four. The first has the change to the root analysis_options followed by a straight application of dart fix everywhere (sometimes multiple times to catch all the cases, see dart-lang/sdk#47398). The second removes trailing spaces added by dart fix. The third fixes analyzer errors introduced by dart fix. The fourth fixes some formatting issues and restores comments removed by dart fix, as well as some other changes to tests to make them pass again.

@Hixie Hixie force-pushed the default_arguments branch 6 times, most recently from ba12c57 to 96d873e Compare October 7, 2021 07:15
@Hixie
Copy link
Contributor Author

Hixie commented Oct 7, 2021

cc @goderbauer and @christopherfujino since this mostly affected the framework and the tool.

@christopherfujino
Copy link
Contributor

My browser can't handle this diff...

goderbauer
goderbauer approved these changes Oct 7, 2021
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.

RSLGTM (I did not look at every single line)

packages/flutter/lib/src/cupertino/context_menu.dart Outdated
@@ -1171,31 +1170,31 @@ class _ContextMenuSheet extends StatelessWidget {
return _orientation == Orientation.portrait
? <Widget>[
const Spacer(
flex: 1,

Copy link
Member

Choose a reason for hiding this comment

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

nit: remove these empty lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, i cleaned up all the ones i saw, but missed these. i'll do a more thorough search.

@christopherfujino
Copy link
Contributor

The ci.yaml validation:

GitHub Error: Requested Resource was Not Found

Is a race condition where we tried to look up the .ci.yaml file from the commit before GitHub had propagated it to rawgithubusercontent. Unfortunately you will need to push a new commit to re-run this check.

@Hixie
Copy link
Contributor Author

Hixie commented Oct 7, 2021

Google testing failure is because #91309 hasn't rolled into google3 yet so this PR causes a bunch of warnings to trigger that won't trigger once that's rolled in.

@Hixie Hixie force-pushed the default_arguments branch from 96d873e to f1dbcc0 Compare October 7, 2021 21:11
christopherfujino
christopherfujino approved these changes Oct 7, 2021
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@Hixie Hixie force-pushed the default_arguments branch from f1dbcc0 to 4e752f7 Compare October 7, 2021 22:43
@fluttergithubbot
Copy link
Contributor

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

  • This commit is not mergeable and has conflicts. Please rebase your PR and fix all the conflicts.

@Hixie Hixie force-pushed the default_arguments branch from 4e752f7 to afc13c1 Compare October 8, 2021 00:15
@fluttergithubbot
Copy link
Contributor

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

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

@fluttergithubbot fluttergithubbot merged commit 5fd259b into flutter:master Oct 8, 2021
zanderso added a commit that referenced this pull request Oct 8, 2021
@zanderso
Revert "Enable avoid_redundant_argument_values lint (#91409)"
2441527 
This reverts commit 5fd259b.
@zanderso zanderso mentioned this pull request Oct 8, 2021
Merged
zanderso added a commit that referenced this pull request Oct 8, 2021
@zanderso
Revert "Enable avoid_redundant_argument_values lint (#91409)" (#91461)
b9d2177 
This reverts commit 5fd259b.
Hixie added a commit to Hixie/flutter that referenced this pull request Oct 8, 2021
@Hixie
Enable avoid_redundant_argument_values lint (flutter#91409)
9eb8067
@Hixie Hixie mentioned this pull request Oct 8, 2021
Merged
christopherfujino pushed a commit that referenced this pull request Oct 8, 2021
@Hixie
Enable avoid_redundant_argument_values lint (#91409) (#91462)
61a0add
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Oct 29, 2021
@Hixie @clocksmith
Enable avoid_redundant_argument_values lint (flutter#91409)
b4c7947
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Oct 29, 2021
@zanderso @clocksmith
Revert "Enable avoid_redundant_argument_values lint (flutter#91409)" (f…
7a70454 
…lutter#91461)

This reverts commit 5fd259b.
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Oct 29, 2021
@Hixie @clocksmith
Enable avoid_redundant_argument_values lint (flutter#91409) (flutter#…
53818df 
…91462)
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

+1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) a: animation Animation APIs 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: contributor-productivity Team-specific productivity, code health, technical debt. d: examples Sample code and demos f: cupertino flutter/packages/flutter/cupertino repository f: focus Focus traversal, gaining or losing focus f: gestures flutter/packages/flutter/gestures repository. f: integration_test The flutter/packages/integration_test plugin f: material design flutter/packages/flutter/material repository. f: routes Navigator, Router, and related APIs. f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: 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.
4 participants
@Hixie @christopherfujino @fluttergithubbot @goderbauer