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 2 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
12 changes: 7 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 @@ -154,20 +154,22 @@ 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 = "\n".join(["Co-authored-by: %s" % a for a in distinct_authors])
Copy link
Member Author

@dbtsai dbtsai Aug 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

For single author commit, should we do Authored-by instead of Co-authored-by?

Or we can have the first author as Authored-by like the following.

Authored-by: David <[email protected]>
Co-authored-by: Bill <[email protected]>

Copy link
Member

@felixcheung felixcheung Aug 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

or leave it as before Author: to be safe?

Copy link
Member

@srowen srowen Aug 4, 2018

Choose a reason for hiding this comment

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

Do these particular keys have a meaning to github or other tools? if "Author:" is what something is looking for, we'd want to leave it, I presume. But adding more lines for co-authors seems fine. I would have just listed multiple "Author:" lines, but sounds like there's another convention around "Co-authored-by:" that another tool looks for?

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.

Seems that those keys are widely used in Linux community for a long time, and Github adopted Co-authored-by to include the work of cu-authoers in the profile contributions graph and the repository's statistics.

This has to be in the end of the message, and Github will just ignore those keys that are not recognized.

If we want to keep both multiple lines of Author: and Co-authored-by:, the information will be duplicated twice. I would like to purpose to just keep Co-authored-by:. For single developer commit, we can just use Authored-by: for consistency.

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