Skip to content
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.

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

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

Open
gr2m opened this issue Nov 10, 2021 · 10 comments
Open

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

gr2m opened this issue Nov 10, 2021 · 10 comments

Comments

@gr2m
Copy link
Contributor

gr2m commented Nov 10, 2021

This is a suggest to lower the barrier for creating remediation commits (depends on #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?

@Willmish
Copy link

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?

@ryjones
Copy link

ryjones commented Mar 17, 2023

@Willmish How would the README be better written?

@Willmish
Copy link

@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: Green-Software-Foundation/carbon-aware-sdk#309 (comment)

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

@ryjones
Copy link

ryjones commented Mar 17, 2023

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

@Willmish
Copy link

@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
Copy link

This comment #171 (comment) by @gr2m also captures what I think the README / docs should include

@brianwarner
Copy link
Contributor

@ryjones want to assign that to me?

@ryjones
Copy link

ryjones commented Mar 17, 2023

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

@Willmish
Copy link

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

@ryjones
Copy link

ryjones commented Apr 26, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants