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.

Sign up for GitHub

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

Jump to bottom

dev-tools: fix pylint in cherrypick_pr #3770

Merged
merged 3 commits into from
Mar 24, 2017
Merged

dev-tools: fix pylint in cherrypick_pr #3770

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd12e47
dev-tools: fix pylint in cherrypick_pr
Mar 17, 2017
6f9156c
dev-tools: make cherrypick_pr compatible with python3
Mar 20, 2017
302b848
cherrypick_pr: go back to using raw_input
Mar 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 21 additions & 20 deletions dev-tools/cherrypick_pr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
#!/usr/bin/env python
"""Cherry pick and backport a PR"""

import sys
import json
import argparse
from os.path import expanduser
import re
from subprocess import check_call, call, check_output
import requests
import re

"""
Example usage:
Expand Down Expand Up @@ -35,6 +36,7 @@ PR.


def main():
"""Main"""
parser = argparse.ArgumentParser(
description="Creates a PR for merging two branches")
parser.add_argument("to_branch",
Expand All @@ -43,7 +45,7 @@ def main():
help="The PR number being merged (e.g. 2345)")
parser.add_argument("commit_hashes", metavar="hash", nargs="+",
help="The commit hashes to cherry pick." +
" You can specify multiple.")
" You can specify multiple.")
parser.add_argument("--yes", action="store_true",
help="Assume yes. Warning: discards local changes.")
parser.add_argument("--continue", action="store_true",
Expand All @@ -52,16 +54,16 @@ def main():
help="From branch")
parser.add_argument("--create_pr", action="store_true",
help="Create a PR using the Github API " +
"(requires token in ~/.github_token)")
"(requires token in ~/.github_token)")
args = parser.parse_args()

print args
print(args)

tmp_branch = "backport_{}_{}".format(args.pr_number, args.to_branch)

if not vars(args)["continue"]:
if not args.yes and raw_input("This will destroy all local changes. " +
"Continue? [y/n]: ") != "y":
if not args.yes and input("This will destroy all local changes. " +
"Continue? [y/n]: ") != "y":
return 1
check_call("git reset --hard", shell=True)
check_call("git clean -df", shell=True)
Expand Down Expand Up @@ -94,7 +96,7 @@ def main():
return 1

print("Ready to push branch.")
remote = raw_input("To which remote should I push? (your fork): ")
remote = input("To which remote should I push? (your fork): ")
Copy link
Member

Choose a reason for hiding this comment

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

Will input instead of raw_input work in python 2.7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, that's a risky one I'll revert it.

call("git push {} :{} > /dev/null".format(remote, tmp_branch),
shell=True)
check_call("git push --set-upstream {} {}"
Expand All @@ -106,33 +108,32 @@ def main():
else:
token = open(expanduser("~/.github_token"), "r").read().strip()
base = "https://api.github.com/repos/elastic/beats"
s = requests.Session()
s.headers.update({"Authorization": "token " + token})
session = requests.Session()
Copy link
Member

Choose a reason for hiding this comment

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

+1 on having more descriptive variable names

session.headers.update({"Authorization": "token " + token})

original_pr = s.get(base+"/pulls/"+args.pr_number).json()
original_pr = session.get(base + "/pulls/" + args.pr_number).json()

# get the github username from the remote where we pushed
remote_url = check_output("git remote get-url {}".format(remote),
shell=True)
remote_user = re.search("github.com:(.+)/beats", remote_url).group(1)

# create PR
r = s.post(base+"/pulls", json=dict(
title="Cherry-pick to {}: {}"
.format(args.to_branch, original_pr["title"]),
request = session.post(base + "/pulls", json=dict(
title="Cherry-pick to {}: {}".format(args.to_branch, original_pr["title"]),
head=remote_user + ":" + tmp_branch,
base=args.to_branch,
body="Cherry-pick of PR #{} to {} branch. Original message: \n\n{}"
.format(args.pr_number, args.to_branch, original_pr["body"])
.format(args.pr_number, args.to_branch, original_pr["body"])
))
if r.status_code > 299:
print("Creating PR failed: {}".format(r.json()))
if request.status_code > 299:
print("Creating PR failed: {}".format(request.json()))
sys.exit(1)
new_pr = r.json()
new_pr = request.json()

# add labels
s.post(base+"/issues/{}/labels".format(new_pr["number"]),
json=["backport", "review"])
session.post(
base + "/issues/{}/labels".format(new_pr["number"]), json=["backport", "review"])
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably have different settings in the editor, because this version yields an pep error here for me (line too long (93 > 79 characters)). I think I am just using the default pep settings.

We didn't yet impose pep/flake in the CI so far because they can be annoying sometimes and the python code that we have is generally not mission critical. Perhaps it's worth discussing this again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We pass it --max-line-length 120 in the check target, I'll adjust my settings to that. Thanks


print("\nDone. PR created: {}".format(new_pr["html_url"]))
print("Please go and check it and add the review tags")
Expand Down