dcoapp / app

GitHub App that enforces the Developer Certificate of Origin (DCO) on Pull Requests
https://github.com/apps/dco
ISC License
298 stars 75 forks source link

Add more verbose error messages for incorrect or missing signoffs #19

Closed hiimbex closed 7 years ago

hiimbex commented 7 years ago

The first 2 commits of this PR update the .gitignore to not include the .env file and update to probot 0.8.0 and use the latest standards (ie (event, context) => context, integration => app).

This PR adds more verbose error messages if someone does not have a DCO sign off. it modifies lib/dco.js to return true if there is a sign off or an error message from the getError() method. It then checks if a string was returned and if so it adds that to the defaults.failure.description (checking to make sure it isn't already added in the case of multiple commits with incorrect sign offs).

Closes #18. @bkeepers I really didn't even think that the bot would be running on its own repo and I dug myself a bit of a hole lol

hiimbex commented 7 years ago

@bkeepers πŸ’― for linking how to fix things as well. I'd love for this bot to not comment and be as simple as setting a status, but there seems to be a lot of information we need to convey πŸ€”

hiimbex commented 7 years ago

@bkeepers I implemented the changes we discussed using the regex. One interesting result is since someone could have multiple incorrectly authored/emailed sign-offs (I had many for testing purposes), I added a check to only include the first incorrect one here. This prevents us from going over the API defined limit for the length of the description field, which interestingly enough is only 1024 bytes.

I updated target_url to redirect to https://github.com/probot/dco/#how-it-works. I also think this PR can be merged in on its own, without updating that section. And, considering I have been unsuccessful in rebasing and actually singing off on all these commits, I don't think I'm particularly qualified to write that section πŸ˜…

hiimbex commented 7 years ago

Memorializing a conversation with @bkeepers, we're gonna pull the logic out from lib/dco.js for improved readability and to avoid checking if the method returns a string

I'm going to continue banging my head against my own commit sign-offs

hiimbex commented 7 years ago

code updated and dco finally fixed but not sure what to do about the tests πŸ™‡β€β™€οΈ

cc/ @bkeepers

hiimbex commented 7 years ago

Moved back to an extracted function that would return the DCO object that is passed into the GitHub API. For testing I compared JSON.stringify'd versions of the objects to verify they contained the correct content (not sure if there's a better way to verify that two objects have the same sets of property names and values).

(You'd think @bkeepers would get the hint by the 5th comment πŸ˜œπŸ™πŸΌ)

jsquyres commented 7 years ago

Sweet -- thanks for doing this!

Does this mean that if we're using the github-hosted DCO app integration, we get this new feature automatically?

hiimbex commented 7 years ago

@jsquyres yes! It is completely automatic. You should see it in action the next time a sign off fails ☺️

github-actions[bot] commented 2 years ago

:tada: This PR is included in version 1.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: