DCO bot fails when checking commits from accepted change

[dpordomingo]

When using the GH interface to accept a change DCO bot rejects the automatically generated commit even when the Signed-off-by message was correct.

In my opinion, Co-Authored-By message (automatically introduced by GitHub UI) should be ignored.

[hiimbex]

@dpordomingo is the PR public/can you link it? If not can you show the DCO's status/message on that commit?

[dpordomingo]

Hi, many thanks for reviewing this.

This PR: src-d/landing#350
Contains this commit src-d/landing@9b406ac, rejected by DCO bot, but its commit message is:

Author: David <rizome.es@gmail.com>
Commit: GitHub <noreply@github.com>

    Update README.md
    
    Signed-off-by: David Pordomingo <david.pordomingo.f@gmail.com>
    
    Co-Authored-By: dpordomingo <david.pordomingo.f@gmail.com>

You can compare it with this other PR: src-d/landing#340
Containing this commit src-d/landing@ac64462, approved by DCO bot, with the following commit message:

Author: David Pordomingo <David.Pordomingo.F@gmail.com>
Commit: David Pordomingo <David.Pordomingo.F@gmail.com>

    Fix non-https links
    
    Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

[hiimbex]

Actually the issue is not with the co-authored by line, but the actual sign-off. If you look at the details of the check run (https://github.com/src-d/landing/pull/350/checks?check_run_id=52447545), you'll see:

Commit sha: 9b406ac, Author: David, Committer: GitHub; Expected "David rizome.es@gmail.com", but got "David Pordomingo david.pordomingo.f@gmail.com".

You can see this in the details you provided as well:

Author: David rizome.es@gmail.com
Commit: GitHub noreply@github.com

vs

Author: David Pordomingo David.Pordomingo.F@gmail.com
Commit: David Pordomingo David.Pordomingo.F@gmail.com

I think this may be due to how suggested changes works in which "GitHub" is credited as the committer and "David" aka rizome (not you) is credited as the author. In either case the sign-off does not match David aka rizome, who introduced the suggested change.

I'm not sure whether the suggested change commit credit should go to the suggestor or the one who accepts the suggestion. However, for the time being, the information GitHub gives us on that PR only tells us about rizome and that the commit was maybe in the GitHub UI, so we expect rizome's sign-off rather than yours.

GitHub

[RFC] Update README.md by dpordomingo · Pull Request #350 · src-d/landing

Do not merge; acts as an example for #102

[hiimbex]

To be clear, the DCO currently checks for a sign-off on only the first Signed-off-by line of a commit. It compares that name and email to match either the committer or the author. If a match is not found, it will fail. In this case the signed-off-by line contained David Pordomingo <david.pordomingo.f@gmail.com>, however the author and committer were Author: David <rizome.es@gmail.com>; Commit: GitHub <noreply@github.com> so there was no match.

[dpordomingo]

imho DCO bot should be compatible with GitHub workflow, so it should avoid rejecting commits that are introduced by a normal GitHub workflow and that can not be modified by the end user.

I wonder if it would be a good idea to ignore commits that came from GitHub <noreply@github.com> (which are signed with the GitHub GPG key) until GitHub is able to properly sign-off accepted suggestions, or the DCO bot is able to validate those ones.

PS. In this other PR #14, it was also modified DCO bot to avoid checking for sig-off message on merges.

[marnovo]

@dpordomingo has a very good point here. Otherwise it becomes more of a hassle than a convenience, plus all the false positives…

[hiimbex]

PRs welcome; however, in this specific instance, I do think that fault is the awkward GitHub flow of suggested changes. Feel free to contact support if you'd like to see that fixed.

[dpordomingo]

Thanks @hiimbex for your time.
Should I understand that you see reasonable the feature of ignoring commits that came from GitHub noreply@github.com, so you would accept a PR implementing it?
If so, I'd give it a try.

[hiimbex]

Sorry, I wasn't very clear. I was suggesting we could support the Co-authored-by line as a sign-off, (opt in).

I'm pretty open to a variety of new features as long as they are opt-in via configs, so that the app can fit as many people's use cases as possible. Personally, I don't think we should completely ignore commits that happen through the GitHub UI, there's a lot of other scenarios in which I still think it reasonable to expect a sign-off; however, I'm still open to a PR for that as long as it's hyper clear what is happening and what enabling the config option does (through naming/docs).

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?