Skip to content

PatchSeries: add a signed-off-by line for the original author #1

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

PatchSeries: add a signed-off-by line for the original author #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 10 additions & 1 deletion lib/patch-series.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,15 @@ export class PatchSeries {

logger?.log(`\nheader:\n${header}\n**************`);

mails[i] = header + "\n\nFrom: " + authorMatch[2] + match[2];
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);
}
Comment on lines +493 to +499
Copy link
Collaborator

@softworkz softworkz Sep 20, 2022

Choose a reason for hiding this comment

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

Could you show an example of what this is trying to solve?

Copy link
Collaborator Author

@rcombs rcombs Sep 20, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@softworkz softworkz Sep 20, 2022

Choose a reason for hiding this comment

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

Wouldn't it be easier to insert that line right away instead of going through format-patch --signoff and replacing later?


mails[i] = header + "\n\nFrom: " + authorMatch[2] + body;


});
Expand Down Expand Up @@ -938,6 +946,7 @@ export class PatchSeries {
const args = [
"format-patch", "--thread", "--stdout", "--signature=ffmpeg-codebot",
"--add-header=Fcc: Sent",
"--signoff", "--zero-commit",
Copy link
Collaborator

@softworkz softworkz Sep 20, 2022

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

@rcombs rcombs Sep 20, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@softworkz softworkz Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

* Add a Pull Request comment on a specific commit
*
* @param {string} pullRequestURL the Pull Request to comment on
* @param {string} commit the hash of the commit to comment on
* @param {string} comment the comment
* @returns the comment ID and the URL to the comment
*/
public async addPRCommitComment(pullRequestURL: string,
commit: string,
gitWorkDir: string | undefined,
comment: string):

"--base", mergeBase, this.project.to,
].concat(PatchSeries.generateSingletonHeaders());
this.project.cc.map((email) => {
Expand Down