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

Enhancements of the cherrypick PRs script #3848

Merged
merged 2 commits into from Mar 30, 2017

Conversation

monicasarbu
Copy link
Contributor

@monicasarbu monicasarbu commented Mar 29, 2017

This PR includes the following changes:

  • add the original PR in the title
  • delete the needs_backport label from the original PR once the backport PR was opened

@@ -135,6 +135,9 @@ def main():
session.post(
base + "/issues/{}/labels".format(new_pr["number"]), json=["backport", "review"])

# remove needs backport label from the original PR
session.delete(base + "/issues/{}/labels/needs_backport".format(args.pr_number))
Copy link
Member

Choose a reason for hiding this comment

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

My only two concern with this is that the label should only be removed, when the PR is merged. And it also does not work if their are backports needed to 5.3 and 5.4

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree it's not ideal. In case we need to backport a PR to multiple branches, then it would be nice to enhance the script to be able to pass a list of branches instead of one, and then remove the needs_backport, but this is for another PR.

@ruflin ruflin merged commit 644ae11 into elastic:master Mar 30, 2017
@ruflin
Copy link
Member

ruflin commented Mar 30, 2017

LGTM. See my comment above. I think it's more an edge case and more often it works then it doesn't.

@monicasarbu monicasarbu deleted the cherrypick_script_enhancements branch March 30, 2017 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants