Skip to content

clear-pr-merge-commit-message - Preserve "Signed-off-by" lines#9336

Merged
fregante merged 3 commits into
refined-github:mainfrom
agriyakhetarpal:preserve-signed-off-by
May 2, 2026
Merged

clear-pr-merge-commit-message - Preserve "Signed-off-by" lines#9336
fregante merged 3 commits into
refined-github:mainfrom
agriyakhetarpal:preserve-signed-off-by

Conversation

@agriyakhetarpal

@agriyakhetarpal agriyakhetarpal commented May 1, 2026

Copy link
Copy Markdown
Contributor

Hi! This PR addresses one part of #9330 by not removing "Signed-off-by" lines (which are usually required by DCO). I've added some test cases too, which were fairly straightforward.

Test URLs

Tested on Dependabot's PRs opened on one of the projects I help maintain: pyodide/pyodide-cli#60

Screenshot

Squash and merge dialog showing both Co-authored-by and Signed-off-by lines preserved in the Extended description field

@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review May 1, 2026 22:14

@fregante fregante left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@fregante

fregante commented May 2, 2026

Copy link
Copy Markdown
Member

As for the first Co-authored-by line, I always found that to be noise, together with what Copilot does. In a future PR I'd probably like to detect them and drop them on PRs that are already authored by them. What do you think?

@agriyakhetarpal

agriyakhetarpal commented May 2, 2026

Copy link
Copy Markdown
Contributor Author

As for the first Co-authored-by line, I always found that to be noise, together with what Copilot does. In a future PR I'd probably like to detect them and drop them on PRs that are already authored by them. What do you think?

I agree with you, and I will gladly take up your offer to contribute further and put together more PRs! :) Do we have a helper somewhere to find who the author of a PR is?

Edit: I'm guessing yes, since one of the features IIRC, does check and adds "Now you can release this change"

Edit 2: okay, we can probably export this:

function getConversationAuthor(): string | undefined {
return $optional([
'.js-command-palette-pull-body .author', // PR conversation
'[data-testid="issue-body-header-author"]', // Issue conversation
])?.textContent;
}
and then pass the author name into cleanCommitMessage.

@fregante

fregante commented May 2, 2026

Copy link
Copy Markdown
Member

Yes, so I think it's only safe to handle these two bots explicitly. There's no straightforward way to match the email to the user without API. There's only exception would be those "privacy emails" that do include the username. We can also drop those if they match the PR author.

@agriyakhetarpal

Copy link
Copy Markdown
Contributor Author

Okay, sounds great. If I understand correctly, privacy emails of the format digits+username@users.noreply.github.com by extracting username should cover all cases, including Dependabot and Copilot, because I can see Dependabot uses 49699333+dependabot[bot]@users.noreply.github.com. It's the same for pre-commit.ci (pre-commit-ci[bot]), Renovate (renovate[bot]), and automated PRs by github-actions[bot] as well, as they also have privacy emails, AFAICS. So I'd say we don't need any explicit bot names?

@fregante

fregante commented May 2, 2026

Copy link
Copy Markdown
Member

If that's the case, then yes.

  1. Get PR author
  2. Remove Co-authored-by lines that match the PR author

That's all.

@fregante fregante changed the title Preserve "Signed-off-by" lines when clearing commit messages clear-pr-merge-commit-message - Preserve "Signed-off-by" lines May 2, 2026
@fregante fregante merged commit 64a8541 into refined-github:main May 2, 2026
13 checks passed
@agriyakhetarpal agriyakhetarpal deleted the preserve-signed-off-by branch May 2, 2026 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants