PatchSeries: add a signed-off-by line for the original author #1
Conversation
|
Oh, I'm very sorry, I missed this PR somehow. This is the first time I'm seeing this. |
|
Seems my settings were wrong. I didn't get a message about your other PR from today either. |
| const args = [ | ||
| "format-patch", "--thread", "--stdout", "--signature=ffmpeg-codebot", | ||
| "--add-header=Fcc: Sent", | ||
| "--signoff", "--zero-commit", |
There was a problem hiding this comment.
The local git identity where this runs is [email protected], so it would add that to all messages.
What would be the purpose of adding --zero-commit?
There was a problem hiding this comment.
It inserts the local identity, which we later replace with the original author identity before sending.
--zero-commit is because we'll be modifying the commit message, which means any git hash is invalidated, so it's more correct to set it to all-zeroes. This doesn't really matter for most purposes.
There was a problem hiding this comment.
GGG is using commit hashes to relate things together like for example syncing comments on the ML back to the GitHub PR.
For example here:
gitgitgadget/lib/github-glue.ts
Lines 190 to 200 in fbf571c
| let body = match[2]; | ||
| if (thisAuthor) { | ||
| let senderSignoff = `\nSigned-off-by: ${thisAuthor}\n`; | ||
| let authorSignoff = `\nSigned-off-by: ${authorMatch[2]}\n`; | ||
| let replaceText = (body.indexOf(authorSignoff) == -1) ? '\n' : authorSignoff; | ||
| body = body.replace(senderSignoff, replaceText); | ||
| } |
There was a problem hiding this comment.
Could you show an example of what this is trying to solve?
There was a problem hiding this comment.
If the author already signed-off on the commit, then the signoff inserted by format-patch is dropped. If they didn't, the inserted signoff is changed to refer to the author.
There was a problem hiding this comment.
Wouldn't it be easier to insert that line right away instead of going through format-patch --signoff and replacing later?
2 participants
I don't have a local setup to test this, but in theory it ought to work.