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

feat: add support for milestone, assignees and requested reviewers #394

Merged
merged 7 commits into from
Nov 5, 2023

Conversation

jschmid1
Copy link
Contributor

@jschmid1 jschmid1 commented Nov 2, 2023

@korthout This patch intends to add three new config options:

  1. copy_milestone

    Copies the milestone from the original PR to all its backport PRs.

  2. copy_requested_reviewers

    Assigns the same requested reviewers to the backport PRs as were on the original PR.

  3. copy_assignees

    Assigns the same individuals to the backport PRs as were assigned to the original PR.

All of these options are disabled by default.

Addresses: #2

@korthout
Copy link
Owner

korthout commented Nov 2, 2023

Wow 😲 thanks for this awesome contribution. I'll give a proper review for this in the coming days, but it looks super useful!

Whether this is the best ordering is subjective, but I prefer a specific
ordering like this until I see a better one (perhaps by categories or
so).
The inputs expect `true` to enable the copying behavior. It wasn't
really clear from the action.yml nor from the readme that these are
boolean flags. By specifying the default as `false` this becomes clear.

Ultimately, an empty input would still disable the copying behavior, but
that's less relevant than the knowledge that this is a boolean flag.

This change doesn't affect the usage or implementation.
I'd like to keep the descriptions of the inputs aligned where
possible/useful to avoid confusion about what they do.
The current implementation is not aligned with this description. It
copies the currently open review requests and does not request reviews
from those that already reviewed the original pull request. Let's adjust
the name and the description to fit what it does: copy the requested
reviewers not the actual reviewers.
We can log the entire response for any of the failed requests. They may
contain valuable info.
Copy link
Owner

@korthout korthout left a comment

Choose a reason for hiding this comment

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

Thanks again @jschmid1 for these wonderful additions! 🙌

Instead of a review where you have to make the adjustments, I've taken the liberty to make the changes on your pull request directly. I hope that's okay with you. 🙇

The biggest change is that I've adjusted copy_reviewers to `copy_requested_reviewers, as this better fits the behavior.

I'll run some quick tests on this now, and then I think it can be merged and released.

Copy link
Owner

@korthout korthout left a comment

Choose a reason for hiding this comment

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

Successfully tested and changes look good! Time to merge and release as part of 2.1.0 🎉

Thanks again for your contribution @jschmid1 🚀

@korthout korthout merged commit c5eeb33 into korthout:main Nov 5, 2023
2 checks passed
@korthout korthout changed the title feat: add support for milestone, assignees and reviewers feat: add support for milestone, assignees and requested reviewers Nov 5, 2023
@jschmid1
Copy link
Contributor Author

jschmid1 commented Nov 5, 2023

Thanks for the quick response! Lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants