Read authorship information from last commit when squashing#152
Read authorship information from last commit when squashing#152kellda wants to merge 1 commit intorust-lang:masterfrom
Conversation
|
It might be slightly better to use the authorship information of the first commit, since sometimes the reviewer will add a commit at the end to fix CI, etc. |
homu/main.py
Outdated
| commiter_name = subprocess.check_output( | ||
| git_cmd('log', '-1', '--format="%an"', | ||
| state.head_sha)).decode('ascii').strip() | ||
| commiter_email = subprocess.check_output( | ||
| git_cmd('log', '-1', '--format="%ae"', | ||
| state.head_sha)).decode('ascii').strip() |
There was a problem hiding this comment.
AFAICT, these are actually getting the author's info, not the committer's, so should the variables be renamed?
|
Do you have an idea of how to get the first commit ? |
|
There might be a better way, but it looks like |
489bec3 to
bba1eda
Compare
|
Should be good now. |
| 'user.name=' + git_cfg['name'], | ||
| 'user.name=' + author_name, | ||
| '-c', | ||
| 'user.email=' + git_cfg['email'], | ||
| 'user.email=' + author_email, |
There was a problem hiding this comment.
I'm not sure, I'll check. Probably not since we already give a list of arguments and not a single command line.
There was a problem hiding this comment.
No, it doesn't: arguments are passed as-is. Also note that squash_msg isn't escaped either.
|
I'd love to get this issue fixed so bors doesn't take credit for people's commits :) |
| if state.squash: | ||
| try: | ||
| first_commit = subprocess.check_output( | ||
| git_cmd('cherry', 'origin/master', |
There was a problem hiding this comment.
We ran into this again in clippy recently so it'd be nice to take another look at this (rust-lang/rust-clippy#8356)
I believe this should be merge_base_sha rather than 'origin/master'
|
@camelid This sounds like an important fix even 4 years after the issue has been reported. Can it be reviewed and finalized? |
Fixes #136.
This only change behaviour for
@bors squash. With the current implementation, bors will still take credit when configured to keep history linear or do autosquashes.