Skip to content

Fix globs in runIf configurations. #122630

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 2 commits into from
Mar 14, 2023
Merged

Fix globs in runIf configurations. #122630

auto-submit merged 2 commits into from
Mar 14, 2023

Conversation

@godofredoc
Copy link
Contributor

@godofredoc godofredoc commented Mar 14, 2023

Several tests have not been running on presubmit after the glob behavior was fixed in the cocoon backend. This change updates runIf configs to make consistent use of globs.

Bug: #122157

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.

@godofredoc
Fix globs in runIf configurations.
d6beacd 
Several tests have not been running on presubmit after the glob behavior
was fixed in the cocoon backend. This change updates runIf configs to
make consistent use of globs.

Bug: #122157
@godofredoc godofredoc requested a review from keyonghan as a code owner March 14, 2023 19:48
@christopherfujino
Copy link
Contributor

christopherfujino commented Mar 14, 2023

cc @keyonghan, I thought #122157 was fixed by flutter/cocoon#2516. Were ALL of these runIf conditions really wrong?

@jmagman
Copy link
Member

jmagman commented Mar 14, 2023

Cocoon backend glob fix was flutter/cocoon#2506

@christopherfujino
Copy link
Contributor

christopherfujino commented Mar 14, 2023

Cocoon backend glob fix was flutter/cocoon#2506

So all of these runIf conditions were wrong for 2 weeks because they didn't actually glob and we added a $ to the end of the regex?

@godofredoc
Copy link
Contributor Author

godofredoc commented Mar 14, 2023

ll of these runIf conditions were wrong for 2 weeks because they didn't actually glob and we added a $ to the end of the regex

That's correct, we created that fix because the builds using the glob were not being run correctly but didn't check the configs in .ci.yaml apart from the ones referenced in the bug.

keyonghan
keyonghan approved these changes Mar 14, 2023
View reviewed changes
Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM

christopherfujino
christopherfujino approved these changes Mar 14, 2023
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

@christopherfujino
Copy link
Contributor

christopherfujino commented Mar 14, 2023

ll of these runIf conditions were wrong for 2 weeks because they didn't actually glob and we added a $ to the end of the regex

That's correct, we created that fix because the builds using the glob were not being run correctly but didn't check the configs in .ci.yaml apart from the ones referenced in the bug.

Oh interesting. I'm surprised this wasn't more disruptive.

@godofredoc godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 14, 2023
@auto-submit auto-submit bot merged commit 7717839 into flutter:master Mar 14, 2023
@godofredoc godofredoc deleted the fix_glob_runif branch March 15, 2023 00:20
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 15, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 15, 2023
@engine-flutter-autoroll
7717839 Fix globs in runIf configurations. (flutter/flutter#122630)
1bf4c14
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
7717839 Fix globs in runIf configurations. (flutter/flutter#122630)
a4675fd
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
7717839 Fix globs in runIf configurations. (flutter/flutter#122630)
cdc722d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmagman jmagman Awaiting requested review from jmagman

2 more reviewers

@christopherfujino christopherfujino christopherfujino approved these changes

@keyonghan keyonghan keyonghan 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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@godofredoc @christopherfujino @jmagman @keyonghan