DCO (signed-off-by) in commits via web ui

caniszczyk commented 7 years ago

When you use the web ui in GitHub, there's no option to do the equivalent of (git commit -s) which is required as part of adhering to the DCO. The GitHub web ui should support this workflow :)

bkeepers commented 7 years ago

This is the biggest thing that can't be built into an integration.

One question I have about this: The DCO requires sign-off on each commit, so this would have to be a checkbox wherever commits are made (merge button, editor) instead of a user config that says "Add sign-off on all my commits"?

xref https://github.com/probot/dco/issues/8 #48

jparise commented 7 years ago

Would this also imply some level of support for #32?

caniszczyk commented 6 years ago

@bkeepers any chance in hell in this one happening :)?

More LF related projects are using DCO on GitHub (thanks for prbot) and not being able to edit things via the GH UI is pain :) https://github.com/todogroup/gh-issues/issues/50

mattklein123 commented 6 years ago

+1 please!

craigez commented 6 years ago

I just ran into a related issue the other day, the probot app only works on PRs and not direct pushes. If you want pushes and PRs to have the DCO enforced it looks like you need a hook as well :(. I need to check if Org level hooks are available/enforceable.

bkeepers commented 6 years ago

@bkeepers any chance in hell in this one happening :)?

I don't know.

In general, my advice would be to try to standardize practices around the DCO as much as possible, and clearly document all the best practices and edge cases.

I could be a really easy feature to add (checkbox on the commit comment box?), but it also brings up a bunch of other questions: Can it be automatic, or does the user have to take an action every time to communicate their intent? Is it visible on every repository, or is there a repository setting that turns it on? Should there be a setting to require it on all commits? What about scenarios where it's not actually required, like bots https://github.com/probot/dco/issues/21?.

Unless I'm misunderstanding something, you can manually include the Signed-off-by line in the commit comment box, right? I know it's annoying, but I just want to make sure it's possible first. You could use a browser extension to make that quick and easy.

dco

probot app only works on PRs and not direct pushes

It is currently recommended to set DCO as a required status to protect master. Feel free to open an issue on probot/dco if you'd like to see it do something different.

mattklein123 commented 6 years ago

I want to mention https://github.com/todogroup/gh-issues/issues/53 (which I just opened) in the context of this issue. I think it would be really awesome if we could block force pushes to PRs. However, if we do that, making sure that people put DCO onto every commit is super important, otherwise it will destroy a PR where someone forgets. IMO DCO will need to get built directly into GH as a first class thing in order to make the workflow not painful (it's actually pretty painful today).

bkeepers commented 6 years ago

IMO DCO will need to get built directly into GH as a first class thing in order to make the workflow not painful (it's actually pretty painful today).

I'd love to hear what the ideal workflow would be for you. Any examples of tools that do it really well?

mattklein123 commented 6 years ago

I'd love to hear what the ideal workflow would be for you. Any examples of tools that do it really well?

I don't have any examples, as I've just been using DCO now for about a week. But I can tell you my current thoughts based on what I've seen:

Right now the flow is very painful as every commit has to manually have the DCO added. If someone forgets on a single commit, the DCO bot fails, and the only possible resolution for the user is to manually rebase the PR to fix the commit message (which is asking a lot) or to squash and force push with DCO. Either way the review history gets messed up badly (see #53). We can have documentation and also a client side commit hook that we ask people to install, but this is high friction for new/infrequent contributors and people forget. As a reviewer this is my biggest pain point right now.

My optimal baseline flow would look like:

Per #53 have option of blocking force push on PRs once opened to preserve review history.
Have GH optionally enforce DCO (like the bot does) as a server side commit hook. It would check every pushed commit for DCO and fail if not there.
Further optionally, we should be able to ask that the DCO check verifies the email against a verified GH email (see https://github.com/probot/dco/issues/34).

In a perfect world we would also get the ability to:

Have DCO be part of the web commit UI where it blocks me from committing depending on project permissions.
Setup "auto DCO" for my account so GH would just auto sign off all my commits for me for a particular project, whether via web UI or manual push.

I realize the previous two points require a lot of thinking, so would be very happy to just nail the baseline flow first.

htuch commented 6 years ago

+1 on server-side commit hook enforcing DCO (at least for Envoy). This seems a simple change to get us most of the pain out of the way.

jmertic commented 5 years ago

I'd be fine with an extra checkbox below the commit message that lets you do the signoff.

leecalcote commented 5 years ago

I want to enable probot's dco enforcement, but the lack of web ui support will likely cut off a high number of quick edits from communities engaged in my projects.

To me, the ideal functionality here is a GitHub account user preference (configurable per repo or per org) for auto-signing signing commits whether via web UI or manual push (+1 to @mattklein123 and @htuch's suggestion). The lack of web UI support here certainly turns away anyone inclined to quickly contribute markdown edits on projects. This inhibits user contribution to documentation and other non-code artifacts (key for much of @caniszczyk's stewarding and organizing within the CNCF).

mkdolan commented 4 years ago

Has there been any progress on this @bkeepers ? Is there any way we can help move it forward?

caniszczyk commented 4 years ago

@jeffmcaffer may know but right now, you can just manually add the "Signed-off-by: XYZ" as part of the pull request and it works fine

jplevyak commented 4 years ago

+1

Just got burned by this.

ryjones commented 4 years ago

This just burned us, as well, on Hyperledger.

bdougie commented 4 years ago

Would love feedback if this helps: I wrote a GitHub Action to edit commits to a pull request with a signature.

To try it out add the below to your .github/workflows/sign-off.yml in a PR. I have it limited to markdown files, but you can remove that path requirement in the YAML if needed.

https://gist.github.com/bdougie/c31ea9f59d9d049a732a046be017eb28

update: I see there is a DCO bot and this action doesn't work well with this. Some tinkering is still needed.

brianjmurrell commented 4 years ago

@bdougie Doesn't your concept invalidate the point of the SoB? AFAIU, the SoB is supposed to be the person submitting the patch making personal attestations to the validity/origin and right to submit the patch.

As such it seems it needs to be submitted with the patch by the author, not just added at commit time by the SCM server as the SCM doesn't know the origin of the patch and whether it's origin is compatible with the project. It could be copyrighted material after-all and in such a case, the submitter didn't sign it off, GitHub (who really can't/shouldn't) did.

amye commented 4 years ago

Just burned on this one, even with a Signed-off-by in the commit. If you have your email address set to private, it wants the 'noreply' email, which is obviously a thing you completely remember every time.

ryjones commented 4 years ago

I know @dhuseby is working on DID:GIT to try to obviate some of this

arsulegai commented 4 years ago

@ryjones Where do I join discussion on that item in Hyperledger?

rainbowdn commented 4 years ago

I forgot to sign my commit and using git commit -s --amend --no-edit, signs my commit with myusername@users.noreply.github.com. I have my name and email set in git config --global user.name and user.email but still my commit get signed with myusername@users.noreply.github.com, any idea how to solve this?

jauderho commented 2 years ago

This is probably the single most annoying thing when I want to contribute a simple patch as I do not always remember to check if a repo needs DCO before sending a PR.

I end up having to clone to a local repo, fixup and then force push. Really quite disruptive to the flow. It's been years since this issue was first opened. Could we please get an official response, even if it is wontfix (hopefully not)? Thanks.

stormi commented 2 years ago

+1 to this. We opened our docs to external contributions but users usually don't know git. However as a subproject of Xen and the linux foundation, we have to ask for a DCO, which makes the contribution process tedious to say the least.

wmorgan commented 2 years ago

Our workaround in Linkerd is to allow contributors to just leave a comment in the PR saying that they agree to the DCO for the commits. Maintainer are then ok'd to override the bot when this happens. There's nothing magic about the signoff header or this bot, and IMO it is too often used without the context that would actually make it effective.

https://github.com/linkerd/linkerd2/blob/main/CONTRIBUTING.md#developer-certificate-of-origin

ryjones commented 2 years ago

Hi, everyone. This feature is in private beta for testing right now. If you'd like to see it in action, fork a Hyperledger repo of your choice and use the web ui to make an edit

maelvls commented 2 years ago

I just tested it on the hyperledger/fabric repository (since the feature is still private beta and I can't enable it on my own orgs), and it works exactly how I would have expected it to work: when I click "edit this file" in the web UI, the sign-off is automatically added when I am about to commit.

Here is what it looks like:

This is great! ❤️

brianjmurrell commented 2 years ago

Hi, everyone. This feature is in private beta for testing right now. If you'd like to see it in action, fork a Hyperledger repo of your choice and use the web ui to make an edit

How does one go about getting this in their own repos?

mattklein123 commented 2 years ago

Thank you!!! Can you please turn this on for https://github.com/envoyproxy/envoy?

monotek commented 2 years ago

Would also be nice for: https://github.com/prometheus-community/helm-charts 😁

caniszczyk commented 2 years ago

For all of 'https://github.com/containerd' would be great too!

On Sun, Apr 24, 2022 at 11:36 AM André Bauer @.***> wrote:

Would also be nice for: https://github.com/prometheus-community/helm-charts 😁

— Reply to this email directly, view it on GitHub https://github.com/todogroup/gh-issues/issues/50#issuecomment-1107875351, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAPSIK3XEBTWTYK2GRQYPLVGV2BJANCNFSM4DF52MBQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Cheers,

Chris Aniszczyk https://aniszczyk.org

jauderho commented 2 years ago

Hi, everyone. This feature is in private beta for testing right now. If you'd like to see it in action, fork a Hyperledger repo of your choice and use the web ui to make an edit

Would love to have this enabled for all of my repos for testing but testing out a forked Hyperledger repo seems to work well.

davidstaheli commented 2 years ago

Hi, all! I'm a product person at GitHub working on this feature. It's an early prototype that only appears when committing changes to a file. It will soon support signing off on commits made elsewhere, like pull request suggestions and resolving merge conflicts. I just enabled it on the organization and repositories below and would greatly appreciate any feedback you have for how it can be improved for you by responding to this Google Forms survey. Thanks!

mattklein123 commented 2 years ago

@davidstaheli thanks for this. This is a major quality of life improvement.

The main missing thing here is completely blocking pushes that are missing DCO. This is a huge source of breakage and frustration. Will there eventually be a repo option that blocks such pushes so that we can avoid any commits that don't have DCO? (I would also like an option to block force pushes entirely, but that is a different subject.)

davidstaheli commented 2 years ago

Hi, @mattklein123. Yes, but I'm curious what you think about the approach. We're planning a policy for blocking pushes if any of the pushed commits' messages don't match a configurable pattern. For example, people could block commits from being pushed if their messages don't reference a Jira ticket. This could also be used to block commits that aren't signed off by configuring this required pattern for commit messages:

^Signed-off-by: [A-Za-z\s]+\s+<[A-Z0-9a-z._%+-]+@[A-Za-z0-9.-]+.[A-Za-z]{2,64}>$

Since that's still rather tricky, we're looking for ways to make it easier, like having a drop-down on the policy configuration for quickly selecting "Require commits to be signed off" instead of having to enter a pattern like above.

For blocking force pushes, you should be able to use this branch protection setting: Allow force pushes. In repository settings, you could create a branch protection rule (as described in the docs linked to the left) setting the branch matching pattern to * so that force pushes aren't allowed to any branch in the repo. Please let me know if this doesn't meet your needs though. You can always email me at my username@github.com.

mattklein123 commented 2 years ago

Since that's still rather tricky, we're looking for ways to make it easier, like having a drop-down on the policy configuration for quickly selecting "Require commits to be signed off" instead of having to enter a pattern like above.[

This would be really nice, but I think the simple regex implementation won't catch all issues, because in order for the DCO bot to succeed, not only must there be a sign off but it has to match the account also. So IMO DCO has to be a more built in concept in the repo settings in order for you to block pushes that are not correctly signed off based on who is pushing.

For blocking force pushes, you should be able to use this branch protection setting: Allow force pushes. In repository settings, you could create a branch protection rule (as described in the docs linked to the left) setting the branch matching pattern to * so that force pushes aren't allowed to any branch in the repo. Please let me know if this doesn't meet your needs though. You can always email me at my username@github.com.

The problem here is actually pushing to forks. Force pushing effectively breaks GH code review in many fundamental ways (don't even get me started on code review ;)), so we tell people not to do it, but they do it anyway. I just want to completely block force push for all branches and all forks. The forks is really the important one though. Or more specifically, any fork branch with an open PR against the main repo. Does this makes sense? (I have asked for this feature for years fwiw.)

brianjmurrell commented 2 years ago

Force pushing effectively breaks GH code review in many fundamental ways

+1. I would be willing to be less rigid on force pushes though and allow them only until reviewing has started. Once a PR owner has requested reviews, they should no longer be allowed to force push.

But that said, GitHub could still allow force-pushing and incremental reviewing if rather than reviewing commits since your last review, they produced a diff between the sha1 you last reviewed and the current sha1, whether they have linear commits between them or not. There is always a diff between the last sha1 you reviewed and the current one, even if between the last one and the current one there have been force pushes. GH's git server would just have to be smart enough not to GC the dangling/unattached sha1's that represent where somebody last reviewed until they review beyond it.

davidstaheli commented 2 years ago

@mattklein123 and @brianjmurrell, thanks for those points about force pushes to forks and incremental reviewing. Sorry if it's taken so long for these to get attention. I created a feature on our backlog to investigate options and your points are very helpful. Please feel free to @mention me in a TODO Group issue if you create one for this, or you could post it in GitHub's feedback discussions where I'll see it too. But I'll try to stay in touch as I see any action happening on this.

davidstaheli commented 2 years ago

How does one go about getting this in their own repos?

@brianjmurrell and all, please let me know if you'd like this preview feature enabled on other organizations or repos.

maelvls commented 2 years ago

@davidstaheli Can we have this preview feature enabled on the following org:

github.com/cert-manager

Thank you so much.

davidstaheli commented 2 years ago

@maelvls, done.

amzn-changml commented 2 years ago

@davidstaheli Could we also enable this preview in our org?

github.com/o3de

Really appreciate that you're working on this!

davidstaheli commented 2 years ago

@amzn-changml, done!

lizrice commented 2 years ago

@davidstaheli could you turn on this preview for github.com/cilium please? I'm so pleased this feature is coming!

amzn-changml commented 2 years ago

Thanks! I noticed you mentioned adding this feature to review suggestions, which would be really helpful. We regularly hit DCO errors because the suggestion doesn't have sign-off when committed. It's also a pain to manually sign every suggestion.

Would that workflow also detect who made the suggestion and make the sign-off on their behalf (maybe even sign-off when the suggestion is made)?

bwplotka commented 2 years ago

Can we have this on https://github.com/thanos-io organization as well? Thanks!

leecalcote commented 2 years ago

Years in the wait. So pleased to see activity on this. Kindly add: https://github.com/meshery, please.

scheeles commented 2 years ago

@davidstaheli Would be great if you can activate it for https://github.com/kubermatic as well. Thanks!

stormi commented 2 years ago

@davidstaheli Hi. I'm interested in testing it on https://github.com/xcp-ng/. Thanks!

bfirsh commented 2 years ago

@davidstaheli It'd be great to get this feature on https://github.com/replicate/cog . Thank you!

todogroup / gh-issues

DCO (signed-off-by) in commits via web ui #50