From 7924dab2db5cd21f89e447d932874a7f2dfe776e Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 15 Jul 2019 10:59:36 -0500 Subject: [PATCH 1/5] Add support for lead/co-authors to merge script --- dev/merge_arrow_pr.py | 66 ++++++++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index b295ac5e658..ffbb2b2412d 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -28,8 +28,8 @@ # There are several pieces of authorization possibly needed via environment # variables # -# JIRA_USERNAME: your Apache JIRA id -# JIRA_PASSWORD: your Apache JIRA password +# APACHE_JIRA_USERNAME: your Apache JIRA id +# APACHE_JIRA_PASSWORD: your Apache JIRA password # ARROW_GITHUB_API_TOKEN: a GitHub API token to use for API requests (to avoid # rate limiting) @@ -52,6 +52,11 @@ print("Exiting without trying to close the associated JIRA.") sys.exit(1) +# Remote name which points to the GitHub site +PR_REMOTE_NAME = os.environ.get("PR_REMOTE_NAME", "apache") + +# For testing to avoid accidentally pushing to apache +DEBUG = bool(os.environ.get("DEBUG", "0")) # Prefix added to temporary branches BRANCH_PREFIX = "PR_TOOL" @@ -339,18 +344,43 @@ def merge(self, target_ref='master'): distinct_authors = sorted(set(commit_authors), key=lambda x: commit_authors.count(x), reverse=True) - primary_author = distinct_authors[0] + + if len(distinct_authors) > 1: + for i, author in enumerate(distinct_authors): + print("Author {}: {}".format(i + 1, author)) + + primary_author = self.cmd.prompt( + "Enter primary author in the format of \"name \" [%s]: " % + distinct_authors[0]) + commits = run_cmd(['git', 'log', 'HEAD..%s' % pr_branch_name, '--pretty=format:%h <%an> %s']).split("\n\n") + if primary_author == "": + primary_author = distinct_authors[0] + else: + # When primary author is specified manually, de-dup it from author + # list and put it at the head of author list. + distinct_authors = [x for x in distinct_authors + if x != primary_author] + distinct_authors = [primary_author] + distinct_authors merge_message_flags = [] merge_message_flags += ["-m", self.title] if self.body is not None: merge_message_flags += ["-m", self.body] - authors = "\n".join(["Author: %s" % a for a in distinct_authors]) - + committer_name = run_cmd("git config --get user.name").strip() + committer_email = run_cmd("git config --get user.email").strip() + + authors = ("Authored-by:" if len(distinct_authors) == 1 + else "Lead-authored-by:") + authors += " %s" % (distinct_authors.pop(0)) + if len(distinct_authors) > 0: + authors += "\n" + "\n".join(["Co-authored-by: %s" % a + for a in distinct_authors]) + authors += "\n" + "Signed-off-by: %s <%s>" % (committer_name, + committer_email) merge_message_flags += ["-m", authors] if had_conflicts: @@ -371,6 +401,9 @@ def merge(self, target_ref='master'): stripped_message = strip_ci_directives(c).strip() merge_message_flags += ["-m", stripped_message] + if DEBUG: + print("\n".join(merge_message_flags)) + run_cmd(['git', 'commit', '--no-verify', # do not run commit hooks '--author="%s"' % primary_author] + @@ -380,9 +413,13 @@ def merge(self, target_ref='master'): % (target_branch_name, self.git_remote)) try: - run_cmd('git push %s %s:%s' % (self.git_remote, - target_branch_name, - target_ref)) + push_cmd = ('git push %s %s:%s' % (self.git_remote, + target_branch_name, + target_ref)) + if DEBUG: + print(push_cmd) + else: + run_cmd(push_cmd) except Exception as e: clean_up() self.cmd.fail("Exception while pushing: %s" % e) @@ -415,17 +452,17 @@ def get_version_json(version_str): def connect_jira(cmd): # ASF JIRA username - jira_username = os.environ.get("JIRA_USERNAME") + jira_username = os.environ.get("APACHE_JIRA_USERNAME") # ASF JIRA password - jira_password = os.environ.get("JIRA_PASSWORD") + jira_password = os.environ.get("APACHE_JIRA_PASSWORD") if not jira_username: - jira_username = cmd.prompt("Env JIRA_USERNAME not set, " + jira_username = cmd.prompt("Env APACHE_JIRA_USERNAME not set, " "please enter your JIRA username:") if not jira_password: - jira_password = cmd.getpass("Env JIRA_PASSWORD not set, " + jira_password = cmd.getpass("Env APACHE_JIRA_PASSWORD not set, " "please enter " "your JIRA password:") @@ -444,15 +481,12 @@ def cli(): pr_num = input("Which pull request would you like to merge? (e.g. 34): ") - # Remote name which points to the GitHub site - git_remote = os.environ.get("PR_REMOTE_NAME", "apache") - os.chdir(ARROW_HOME) github_api = GitHubAPI(PROJECT_NAME) jira_con = connect_jira(cmd) - pr = PullRequest(cmd, github_api, git_remote, jira_con, pr_num) + pr = PullRequest(cmd, github_api, PR_REMOTE_NAME, jira_con, pr_num) if pr.is_merged: print("Pull request %s has already been merged") From 0ebc17ec2d65ccce0faeab6676a30b8481b7bc10 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 15 Jul 2019 11:02:56 -0500 Subject: [PATCH 2/5] Add debugging output --- dev/merge_arrow_pr.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index ffbb2b2412d..3ff414b2a38 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -58,6 +58,11 @@ # For testing to avoid accidentally pushing to apache DEBUG = bool(os.environ.get("DEBUG", "0")) + +if DEBUG: + print("**************** DEBUGGING ****************") + + # Prefix added to temporary branches BRANCH_PREFIX = "PR_TOOL" JIRA_API_BASE = "https://issues.apache.org/jira" From 6d65ceb3f37826f3862dac5ad74991645856c935 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Mon, 15 Jul 2019 13:46:46 -0500 Subject: [PATCH 3/5] Fix DEBUG env variable flag logic [skip ci] --- dev/merge_arrow_pr.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 3ff414b2a38..2ada5e35524 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -56,7 +56,7 @@ PR_REMOTE_NAME = os.environ.get("PR_REMOTE_NAME", "apache") # For testing to avoid accidentally pushing to apache -DEBUG = bool(os.environ.get("DEBUG", "0")) +DEBUG = bool(int(os.environ.get("DEBUG", 0))) if DEBUG: From 74fb732dc7b13d3938b0a6665c83627eafda2d2b Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Tue, 16 Jul 2019 23:04:31 -0500 Subject: [PATCH 4/5] Put authors at end of commit message so GitHub understands them [skip ci] --- dev/merge_arrow_pr.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index 2ada5e35524..bf8feed9ef3 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -386,7 +386,6 @@ def merge(self, target_ref='master'): for a in distinct_authors]) authors += "\n" + "Signed-off-by: %s <%s>" % (committer_name, committer_email) - merge_message_flags += ["-m", authors] if had_conflicts: committer_name = run_cmd("git config --get user.name").strip() @@ -406,6 +405,8 @@ def merge(self, target_ref='master'): stripped_message = strip_ci_directives(c).strip() merge_message_flags += ["-m", stripped_message] + merge_message_flags += ["-m", authors] + if DEBUG: print("\n".join(merge_message_flags)) From 816a137feae2bab65ccf4b29ad14da23033abe8f Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 17 Jul 2019 10:45:11 -0500 Subject: [PATCH 5/5] Do not prompt for lead author if there is only one person involved [skip ci] --- dev/merge_arrow_pr.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/dev/merge_arrow_pr.py b/dev/merge_arrow_pr.py index bf8feed9ef3..623e3352ae5 100755 --- a/dev/merge_arrow_pr.py +++ b/dev/merge_arrow_pr.py @@ -350,25 +350,29 @@ def merge(self, target_ref='master'): key=lambda x: commit_authors.count(x), reverse=True) + for i, author in enumerate(distinct_authors): + print("Author {}: {}".format(i + 1, author)) + if len(distinct_authors) > 1: - for i, author in enumerate(distinct_authors): - print("Author {}: {}".format(i + 1, author)) + primary_author = self.cmd.prompt( + "Enter primary author in the format of " + "\"name \" [%s]: " % distinct_authors[0]) - primary_author = self.cmd.prompt( - "Enter primary author in the format of \"name \" [%s]: " % - distinct_authors[0]) + if primary_author == "": + primary_author = distinct_authors[0] + else: + # When primary author is specified manually, de-dup it from + # author list and put it at the head of author list. + distinct_authors = [x for x in distinct_authors + if x != primary_author] + distinct_authors = [primary_author] + distinct_authors + else: + # If there is only one author, do not prompt for a lead author + primary_author = distinct_authors[0] commits = run_cmd(['git', 'log', 'HEAD..%s' % pr_branch_name, '--pretty=format:%h <%an> %s']).split("\n\n") - if primary_author == "": - primary_author = distinct_authors[0] - else: - # When primary author is specified manually, de-dup it from author - # list and put it at the head of author list. - distinct_authors = [x for x in distinct_authors - if x != primary_author] - distinct_authors = [primary_author] + distinct_authors merge_message_flags = [] merge_message_flags += ["-m", self.title]