dev-tools: fix pylint in cherrypick_pr #3770
Conversation
dev-tools/cherrypick_pr
Outdated
| @@ -77,10 +79,10 @@ def main(): | |||
| check_call("git checkout -b {}".format(tmp_branch), shell=True) | |||
| if call("git cherry-pick -x {}".format(" ".join(args.commit_hashes)), | |||
| shell=True) != 0: | |||
| print("Looks like you have cherry-pick errors.") | |||
There was a problem hiding this comment.
This change will break compatibilty with python 3.
There was a problem hiding this comment.
Good catch! Do you prefer using __future__ or ignoring the warning? https://docs.python.org/3.5/library/__future__.html
There was a problem hiding this comment.
I just switched to python3 and restored the parentheses, also renamed raw_input() to input()
There was a problem hiding this comment.
This probably also ties into the PR #3763. See my comment here: #3763 (comment)
| @@ -106,36 +108,35 @@ 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() | |||
There was a problem hiding this comment.
+1 on having more descriptive variable names
|
Removed the |
dev-tools/cherrypick_pr
Outdated
| @@ -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): ") | |||
There was a problem hiding this comment.
Will input instead of raw_input work in python 2.7?
There was a problem hiding this comment.
You're right, that's a risky one I'll revert it.
| s.post(base+"/issues/{}/labels".format(new_pr["number"]), | ||
| json=["backport", "review"]) | ||
| session.post( | ||
| base + "/issues/{}/labels".format(new_pr["number"]), json=["backport", "review"]) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
We pass it --max-line-length 120 in the check target, I'll adjust my settings to that. Thanks
No description provided.