Skip to content

[SPARK-25018] [Infra] Use Co-authored-by and Signed-off-by git trailer in merge_spark_pr.py #21991

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

Closed
dbtsai wants to merge 11 commits into from
Closed

[SPARK-25018] [Infra] Use Co-authored-by and Signed-off-by git trailer in merge_spark_pr.py #21991

Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2563311
Use co-author-by
dbtsai Aug 3, 2018
6732913
added signed-off-by
dbtsai Aug 3, 2018
fbd8cb4
lead-authored-by
dbtsai Aug 6, 2018
c5cd418
Empty commit to test 'Co-authored-by' and 'Signed-off-by'
HyukjinKwon Aug 6, 2018
7dd0173
Merge pull request #3 from HyukjinKwon/test-script
dbtsai Aug 6, 2018
4500d60
Added new line
dbtsai Aug 6, 2018
a69cfb3
add thrid author
lindblombr Aug 6, 2018
483d182
fix first author bug
dbtsai Aug 6, 2018
cc67c8d
Add manually given primary author at the head of author list.
viirya Aug 6, 2018
11887ae
Merge pull request #4 from viirya/script_pr
dbtsai Aug 6, 2018
272d8fd
De-dup primary author from distinct author list.
viirya Aug 6, 2018
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
18 changes: 13 additions & 5 deletions dev/merge_spark_pr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ def merge_pr(pr_num, target_ref, title, body, pr_repo_desc):
distinct_authors[0])
if primary_author == "":
primary_author = distinct_authors[0]
else:
# When primary author is specified manually, put it at the head of author list.
distinct_authors.insert(0, primary_author)
Copy link
Member Author

@dbtsai dbtsai Aug 6, 2018

Choose a reason for hiding this comment

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

@viirya good catch. When we put it at the head of the author list, do we need to dedupe it again?

Copy link
Member

@viirya viirya Aug 6, 2018

Choose a reason for hiding this comment

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

I gave it a thought, but didn't do it. If you think it is necessary, I will push another commit for it.


commits = run_cmd(['git', 'log', 'HEAD..%s' % pr_branch_name,
'--pretty=format:%h [%an] %s']).split("\n\n")
Expand All @@ -154,20 +157,25 @@ def merge_pr(pr_num, target_ref, title, body, pr_repo_desc):
# to people every time someone creates a public fork of Spark.
merge_message_flags += ["-m", body.replace("@", "")]

authors = "\n".join(["Author: %s" % a for a in distinct_authors])

merge_message_flags += ["-m", authors]
committer_name = run_cmd("git config --get user.name").strip()
committer_email = run_cmd("git config --get user.email").strip()

if had_conflicts:
committer_name = run_cmd("git config --get user.name").strip()
committer_email = run_cmd("git config --get user.email").strip()
message = "This patch had conflicts when merged, resolved by\nCommitter: %s <%s>" % (
committer_name, committer_email)
merge_message_flags += ["-m", message]

# The string "Closes #%s" string is required for GitHub to correctly close the PR
merge_message_flags += ["-m", "Closes #%s from %s." % (pr_num, pr_repo_desc)]

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]

run_cmd(['git', 'commit', '--author="%s"' % primary_author] + merge_message_flags)

continue_maybe("Merge complete (local ref %s). Push to %s?" % (
Expand Down