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.

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

Feature: Add extended-tests label on approval #6097

Closed
etrepum opened this issue May 13, 2024 · 7 comments · Fixed by #6120
Closed

Feature: Add extended-tests label on approval #6097

etrepum opened this issue May 13, 2024 · 7 comments · Fixed by #6120
Assignees
Labels
enhancement Improvement over existing feature infra

Comments

@etrepum
Copy link
Collaborator

etrepum commented May 13, 2024

When a PR is approved, if the label no-extended-tests is not present, add the extended-tests label.

Could be done with an workflow that uses the gh CLI directly or call an action such as labeler and use a condition to run it only when the PR is approved (and maybe if it touches any of public packages source file, the playground, or a package/package-lock file)

Follow-up to #6080

@etrepum etrepum added the enhancement Improvement over existing feature label May 13, 2024
@StyleT
Copy link
Contributor

StyleT commented May 13, 2024

I think it may better to utilize GH merge queues, in addition to manual triggers/labels:

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request-with-a-merge-queue

It’s also discussed here: https://github.com/orgs/community/discussions/53922

@etrepum
Copy link
Collaborator Author

etrepum commented May 13, 2024

Given that I don't have the authorization to use labels or approvals, I don't have a strong opinion either way. I just thought I should add the follow-up issue based on what was discussed on the workflow PR so it doesn't get forgotten, since at the moment there is an extra step in reviews. As long as it's easy and efficient for the people who are doing the reviews (and not error-prone), I will be happy with it.

We should probably also add a few words to the maintainers guide with how the review process works, to communicate the process change to existing maintainers and to make onboarding a new maintainer more efficient.

@StyleT
Copy link
Contributor

StyleT commented May 13, 2024

Thanks for filing this anyway! We shall solve it either way

@StyleT StyleT added the infra label May 13, 2024
@necolas
Copy link
Contributor

necolas commented May 13, 2024

A merge queue is what I first suggested internally too. But didn't realize GitHub now has a built-in version of it!

@Sahejkm Sahejkm self-assigned this May 14, 2024
@Sahejkm
Copy link
Contributor

Sahejkm commented May 14, 2024

following up with OSS team to enable Merge Queue options in for internal meta settings as well, its not visible right now.

@etrepum
Copy link
Collaborator Author

etrepum commented May 18, 2024

@Sahejkm
Copy link
Contributor

Sahejkm commented May 18, 2024

I think it's the same issue as the size limit job to add comments , it doesn't run from forks, and need to use the pull request target event
:(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment