Use commit.comitter instead of commit.author in validation

[bndw]

Uses commit.commiter instead of commit.author in name and email validation. Related to #39

Signed-off-by: bndw benjamindwoodward@gmail.com

[hiimbex]

LGTM! I wonder if instead of adding that conditional which makes the tests pass, we should should update the tests to use a mock committer object instead of that of an author

[bkeepers]

Thanks for opening the pull request!

I think we'll want to consult with someone more familiar with the DCO to check if this is valid.

@mkdolan @caniszczyk: Is there someone you recommend that we could nominate as our ombud for this issue and future questions about the DCO?

[bkeepers]

Now that I'm seeing this, I wonder if it should attempt to check against both committer and author.

For example, I could open a pull request with:

1. commits from me, in which case it could match either committer or author
2. commits from someone else that already included a sign-off, in which case it would match the author
3. commits from someone that didn't include a sign-off but I added a sign-off, in which case it would match the committer

[bndw]

The issue I ran into that prompted this PR was the case where I had signed a commit with one account (committer and email are the same) and later rebased and signed the commit with a different account (only committer). In this case, the author and committer will be different which was unexpected because, in my case, there was only a single commit on a PR and I could visually see it was signed by email address b (but the DCO check was looking for address a)

[bndw]

I agree checking both committer and author seems like the more robust solution. And, really, the above situation I got in is likely one that could be solved outside of this tool, perhaps by updating the README with more details on how this works and common troubleshooting steps.

[hiimbex]

I wonder if it should attempt to check against both committer and author.

Yea I mentioned that in the issue too. I'm not sure whether the committer or author should be the default or whether to use both. If we do use both, which makes more sense to display in the error message?

[mkdolan]

I'm not sure I understand enough about how probot works, but the DCO allows for someone to assert that they have the rights to contribute code even if they were not the author. See DCO section (c), also (b).

[bndw]

@bkeepers was there a concrete change you were looking for here?

[bkeepers]

was there a concrete change you were looking for here?

I think it should match on either the committer or the author. Right now it uses the committer if present, or falls back to the author.

This will currently break if someone pushes a commit that already includes a signoff from another author.

[stale]

Is this still relevant? If so, please comment with any updates or addition details.