app icon indicating copy to clipboard operation
app copied to clipboard

Adding a web link to remediate commits that are not signed off

Open gr2m opened this issue 2 years ago • 10 comments

This is a suggest to lower the barrier for creating remediation commits (depends on https://github.com/probot/dco/pull/147).

As of now, there is no simple way using the GitHub UI to add empty commits with the correct commit message body to sign off for previous commits.

I suggest to implement the following flow

  1. person submits a PR (e.g. fixing a typo using GitHub's UI) without signing their commit
  2. The DCO app adds a comment prompting the user to sign, providing a link to do so
  3. The link redirects to a custom route of the DCO app with a reference to the pull request
  4. The user is asked to authenticate using GitHub
  5. The app creates an empty commit with the correct commit body to sign off on the previous commit, authenticated as the user.
  6. The user gets redirected to the pull request
  7. Because of the newly added remediation commit, DCO can now set the status to success
  8. Bonus: the comment from the DCO app gets hidden as outdated

This could be an opt-in feature.

Note: Adding the comment will require an additional permission which needs to be approved by owners of all installations. But we can add the same link to the check runs we create, where we already document how to fix commits that are not signed off. It will be the simplest solution of all.

What do you think @ryjones @brianwarner @ashleywolf?

gr2m avatar Nov 10 '21 22:11 gr2m

Bumping this up, as I couldn't find any other Issues open/closed on remediation commits.

Could there be a section in the docs added that explains the concept a bit more/example of how to add a remediation commit?

Currently only info availble on it is this: https://github.com/dcoapp/app#individual-remediation-commit-support

However the worklow itself (on failed DCO check) states that the preferred way is by adding a remediation commit:

Preferred method: Commit author adds a DCO remediation commit

A DCO Remediation Commit contains special text in the commit message that applies a missing Signed-off-by line in a subsequent commit. The primary benefit of this method is that the project’s history does not change, and there is no risk of breaking someone else’s work.

These authors can unblock this PR by adding a new commit to this branch with the following text in their commit message:

If this is the preferred method, shouldn't there be a more detailed guide / information on how remediation commits should look like?

Willmish avatar Mar 17 '23 16:03 Willmish

@Willmish How would the README be better written?

ryjones avatar Mar 17 '23 16:03 ryjones

@ryjones - I would suggest either to add the messages that appear in the workflow log when DCO fails (explaining remediation Commit is preferred way of fixing vs Rebasing) and give an example of such Remediation commit. Here are the workflow logs quoted: https://github.com/Green-Software-Foundation/carbon-aware-sdk/issues/309#issuecomment-1474083877

I could not find an alternative source that explains this after DCO error was fixed and passing, so could not refer back to these exact messages (Structure of a remediation commit, and description of why its the preferred method over rebasing). Maybe there is an external documentation explaining this (in GItHub docs?).

So my suggested change would be to, in the section https://github.com/dcoapp/app#individual-remediation-commit-support add the following:

  • Example of how a remediation commit should look like/description of what it should contain
  • Ideally: a source to GitHub docs or other explaining this in general for DCO
  • Explanation why remediation commit is preferred over rebasing

Willmish avatar Mar 17 '23 16:03 Willmish

@Willmish so incorporate the content of the how it works section, as well as adding an example and explanation of remediation versus rebasing

ryjones avatar Mar 17 '23 16:03 ryjones

@ryjones Yup - in my opinion that would be ideal as it would add a reference point for explaining remediation commits, if ever needed to be pointed to in other discussions. This is already very well described when the actual workflow fails and provides a clear guide how to comply with DCO, but I think this message sitting in the repository itself would still be helfpul when there is no failed workflow log message to refer to :D .

Willmish avatar Mar 17 '23 17:03 Willmish

This comment https://github.com/dcoapp/app/pull/171#discussion_r749937389 by @gr2m also captures what I think the README / docs should include

Willmish avatar Mar 17 '23 17:03 Willmish

@ryjones want to assign that to me?

brianwarner avatar Mar 17 '23 17:03 brianwarner

@brianwarner I can't - I have membership, but no commit bit

ryjones avatar Mar 17 '23 17:03 ryjones

Hi @brianwarner @ryjones - just bumping this up (I know this is not critical, doesn't want it to go stale)

Willmish avatar Apr 26 '23 15:04 Willmish

@gr2m may I have the permission to assign this issue to @brianwarner ?

ryjones avatar Apr 26 '23 16:04 ryjones