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

Improve GH Action PR assign + labeling #3584

Merged
merged 1 commit into from
Mar 16, 2021
Merged

Conversation

carlisia
Copy link
Contributor

@carlisia carlisia commented Mar 13, 2021

Signed-off-by: Carlisia [email protected]

Please add a summary of your change

This change removes the synchronize parameter from the pull_request_target GH action in the Auto Assign PR Reviewers workflow. This should avoid the workflow from being triggered on all PR events, such as additional commits, which has the effect of adding additional reviewers (with each additional commit) beyond the current limit of 2. We only want this to be triggered when the PR opens, or when the PR changes from WIP to "ready for review".

It also separates the auto labeling GH action into the "Auto Label PRs" workflow. This one we want to be triggered on every event, so it keeps the synchronize and all other parameters the same.

Thanks to @zubron for pointing these out (in #3573 (comment)). Hopefully this will work.

Does your change fix a particular issue?

Fixes #(#3573)

PS: It doesn't completely fix the entire issue. For example: if the author wants to assign 2 specific reviewers before the PR is created, the GH action will still assign additional reviewers on the "open" event. This is the known bug in the action.

A way to get around this is to remove the originally assigned reviewers after the PR is created, and add the ones you want (assuming they don't match). This "works", but causes unnecessary notifications for the soon-to-be-removed reviewers.

A way to get around wanting to

Please indicate you've done the following:

@carlisia carlisia added the kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes label Mar 13, 2021
@github-actions github-actions bot requested review from dsu-igeek and zubron March 13, 2021 00:47
@carlisia carlisia marked this pull request as draft March 13, 2021 01:22
@carlisia
Copy link
Contributor Author

Changing this to draft. Let me do some tests on a personal repo. It seems that w/o the synchronize param events such as additional commits will cause the GH action to fail bc there's an existing process but no event to handle it. This makes sense now that I think about it.

@carlisia carlisia marked this pull request as ready for review March 15, 2021 16:57
@github-actions github-actions bot requested review from ashish-amarnath and nrb March 15, 2021 16:57
@carlisia
Copy link
Contributor Author

I tested this and it works as expected. Turns out, when on: pull_request_target is used, which is what we have with this action, additional events are not expected to have the synchronize event type to handle them, so it silently does not trigger any processing. on: pull_request, on the other hand, will complain if there isn't the synchronize type to handle additional events.

This is ready for review.

Copy link
Contributor

@zubron zubron left a comment

Choose a reason for hiding this comment

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

Thanks!

@jenting jenting merged commit 6832036 into vmware-tanzu:main Mar 16, 2021
@jenting jenting linked an issue Mar 16, 2021 that may be closed by this pull request
@carlisia carlisia deleted the c-pr branch March 16, 2021 19:02
pradeep288 pushed a commit to pradeep288/velero that referenced this pull request May 4, 2021
Signed-off-by: Carlisia <[email protected]>
Signed-off-by: Pradeep Jigalur <[email protected]>
dharmab pushed a commit to dharmab/velero that referenced this pull request May 25, 2021
ywk253100 pushed a commit to ywk253100/velero that referenced this pull request Jun 29, 2021
gyaozhou pushed a commit to gyaozhou/velero-read that referenced this pull request May 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GH Action bug: If reviewers already assigned, more keep being added
3 participants