-
Notifications
You must be signed in to change notification settings - Fork 29.7k
[github] Add labeler action #126012
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.
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
[github] Add labeler action #126012
Conversation
keyonghan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we validate the actions work as expected
| - accessibility | ||
| - semantics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For all pathContainsLabels, these should be **...**.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading https://github.com/actions/labeler, this is expected. All of these are regexes, so as long as they contain this pattern it'll be matched (same **semanatics** maps to semantics)
| triage: | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any more granular permission that can be used here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, it seems the pull request permission is a catch all. Autosubmit seems to have this same issue where it requires write access to edit the labels on PRs.
I filed a support ticket with GitHub at https://support.github.com/ticket/enterprise/1732/2139514 to see if they can allow smaller ACLs on pull request.
In the interim, Cocoon and the action can both run. If we notice discrepancies, we can update the workflow here. After ~100 PRs, we should be able to remove the logic from Cocoon. |
How do we detect discrepancies between these two logics? |
I plan to manually triage open PRs for the week to make sure the GitHub action is setting all the labels, and not flutter-dashboard. I expect the GitHub action will get priority as it's running directly on GitHub, and has no other work to do (like the webhook subscription does) |
I am wondering if any logs available for the defined action, but a manual look should tell. LGTM as this is not a blocker even something happens with cocoon around. |
keyonghan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@XilaiZhang any idea why this PR isn't getting Google Testing results on it? |
umm looks like this still has notify github set to false, did we go/upate-frob the legacy job? |
|
Last roll out of those jobs was May 1, did we miss deploying a recent fix? |
Right yeah then it wouldn't include cl/529188369 from yesterday which corrects the notify github behavior. |
|
redeployed legacy frob server just now to include yesterday's fix. this was also seen in b/274947171 and impacting other prs. (funny how i was joking in triage yesterday this notifyGithub bug would cause a P0 or P1 soon, and it did prove itself today) |
|
@godofredoc can you take a look? |
|
This is good to merge (clicking on details page shows 0 errors). we can either 1. manually set github status to pass, or 2. push a new commit to propagate the testing result. |
godofredoc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will there be a follow up to remove this functionality from cocoon and adding documentation on how to add new labels?
|
This project will be tracked in #126002. Good point about adding docs. I can't find any existing docs on the wiki, so I'll add some with the new path and send a PSA to contributors who have previously edited the Cocoon files. Once all the Cocoon supported repos have been migrated, I'll send the PR for removing the Cocoon logic. |
flutter/flutter@8c5a1ea...a76dbe4 2023-05-10 [email protected] Manual roll Flutter Engine from 8ca16cba8c38 to 78f41a8f2f06 (2 revisions) (flutter/flutter#126392) 2023-05-10 [email protected] Roll flutter/packages to 0167d83 (flutter/flutter#126427) 2023-05-10 [email protected] Migrate gallery ios tests to build+test (flutter/flutter#111164) 2023-05-09 [email protected] Roll Flutter Engine from 824cd09b8c62 to 8ca16cba8c38 (5 revisions) (flutter/flutter#126360) 2023-05-09 [email protected] Revert "Provide default constraints for M3 dialogs" (flutter/flutter#126355) 2023-05-09 [email protected] Roll Packages from 4800d65 to 1f91710 (8 revisions) (flutter/flutter#126352) 2023-05-09 [email protected] [github] Add labeler action (flutter/flutter#126012) 2023-05-09 [email protected] Fix platformLocation path and search dropping (flutter/flutter#126232) 2023-05-09 [email protected] Roll Flutter Engine from 8d3a8162b3ab to 824cd09b8c62 (10 revisions) (flutter/flutter#126309) 2023-05-09 [email protected] Update FocusNode documentation (flutter/flutter#126331) 2023-05-09 [email protected] Remove dead code (flutter/flutter#126266) 2023-05-09 [email protected] fix AppBar's docs for backgroundColor (flutter/flutter#126194) 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 [email protected],[email protected],[email protected] 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/flutter@8c5a1ea...a76dbe4 2023-05-10 [email protected] Manual roll Flutter Engine from 8ca16cba8c38 to 78f41a8f2f06 (2 revisions) (flutter/flutter#126392) 2023-05-10 [email protected] Roll flutter/packages to 0167d83 (flutter/flutter#126427) 2023-05-10 [email protected] Migrate gallery ios tests to build+test (flutter/flutter#111164) 2023-05-09 [email protected] Roll Flutter Engine from 824cd09b8c62 to 8ca16cba8c38 (5 revisions) (flutter/flutter#126360) 2023-05-09 [email protected] Revert "Provide default constraints for M3 dialogs" (flutter/flutter#126355) 2023-05-09 [email protected] Roll Packages from 4800d65 to 1f91710 (8 revisions) (flutter/flutter#126352) 2023-05-09 [email protected] [github] Add labeler action (flutter/flutter#126012) 2023-05-09 [email protected] Fix platformLocation path and search dropping (flutter/flutter#126232) 2023-05-09 [email protected] Roll Flutter Engine from 8d3a8162b3ab to 824cd09b8c62 (10 revisions) (flutter/flutter#126309) 2023-05-09 [email protected] Update FocusNode documentation (flutter/flutter#126331) 2023-05-09 [email protected] Remove dead code (flutter/flutter#126266) 2023-05-09 [email protected] fix AppBar's docs for backgroundColor (flutter/flutter#126194) 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 [email protected],[email protected],[email protected] 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Part of #126002
Migrate the Cocoon logic for labelling directly into the repo under test
Pre-launch Checklist
///).