Skip to content

[flutter_tools] refactor stringsArg #105032

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
auto-submit merged 4 commits into from
Aug 5, 2022
Merged

[flutter_tools] refactor stringsArg #105032

auto-submit merged 4 commits into from
Aug 5, 2022

Conversation

@amanv8060
Copy link
Contributor

@amanv8060 amanv8060 commented May 31, 2022 •
edited
Loading

Update stringsArg function.

part of #101595

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 31, 2022
@amanv8060
Copy link
Contributor Author

amanv8060 commented May 31, 2022

cc/ @Jasguerrero

Jasguerrero
Jasguerrero reviewed Jun 10, 2022
View reviewed changes
packages/flutter_tools/lib/src/runner/flutter_command.dart Outdated
Copy link
Contributor

@Jasguerrero Jasguerrero Jun 10, 2022

Choose a reason for hiding this comment

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

After this issue #105263 (comment) we are going to change this to bubble up the error when the flag is missing so the behavior we are looking here will be something like arResults?[name]! as List<String>? ?? <String>[]

Copy link
Contributor Author

@amanv8060 amanv8060 Jun 10, 2022

Choose a reason for hiding this comment

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

Sure, Changed to argResults![name]! as List<String>? ?? <String>[];

Copy link
Contributor Author

@amanv8060 amanv8060 Jun 10, 2022

Choose a reason for hiding this comment

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

Did not change it back to arrow operator, based upon styleguide section on arrow operators.

Copy link
Contributor

@christopherfujino christopherfujino Jul 13, 2022

Choose a reason for hiding this comment

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

LGTM

@amanv8060 amanv8060 requested a review from Jasguerrero June 10, 2022 23:45
christopherfujino
christopherfujino reviewed Jun 23, 2022
View reviewed changes
@christopherfujino christopherfujino added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Jun 30, 2022
christopherfujino
christopherfujino approved these changes Jul 13, 2022
View reviewed changes
packages/flutter_tools/lib/src/runner/flutter_command.dart Outdated
Copy link
Contributor

@christopherfujino christopherfujino Jul 13, 2022

Choose a reason for hiding this comment

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

LGTM

Jasguerrero
Jasguerrero approved these changes Jul 21, 2022
View reviewed changes
Copy link
Contributor

@Jasguerrero Jasguerrero left a comment

Choose a reason for hiding this comment

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

LGTM

@amanv8060
Copy link
Contributor Author

amanv8060 commented Jul 26, 2022

@Jasguerrero can you please add the autosubmit label over here ?

@Jasguerrero Jasguerrero added autosubmit Merge PR when tree becomes green via auto submit App and removed waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds labels Jul 26, 2022
@christopherfujino
Copy link
Contributor

christopherfujino commented Aug 4, 2022

@CaseyHillers what should we do here? It's doing the thing where the frob dashboard doesn't hvae the PR, but I get a failure if I try to schedule it.

@CaseyHillers
Copy link
Contributor

CaseyHillers commented Aug 4, 2022

@amanv8060 this PR is 800 commits behind tip of tree. Can you rebase to the latest to fix the stuck Google Testing check?

@auto-submit
Copy link
Contributor

auto-submit bot commented Aug 4, 2022

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

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 4, 2022
@amanv8060
Copy link
Contributor Author

amanv8060 commented Aug 5, 2022 •
edited
Loading

@christopherfujino tests have passed, can you please add the label again?

@CaseyHillers CaseyHillers added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 5, 2022
@auto-submit auto-submit bot merged commit b02f68a into flutter:master Aug 5, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 5, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 5, 2022
@engine-flutter-autoroll
b02f68a [flutter_tools] refactor stringsArg (flutter/flutter#105032)
9a5a712
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 5, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 5, 2022
@engine-flutter-autoroll
b02f68a [flutter_tools] refactor stringsArg (flutter/flutter#105032)
912ca6c
@amanv8060 amanv8060 deleted the stringsArg branch August 5, 2022 18:29
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@amanv8060 @camsim99
[flutter_tools] refactor stringsArg (flutter#105032)
b3ca070
@amanv8060 amanv8060 restored the stringsArg branch January 30, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@christopherfujino christopherfujino christopherfujino approved these changes

@Jasguerrero Jasguerrero Jasguerrero approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App 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

@amanv8060 @christopherfujino @CaseyHillers @Jasguerrero