search

bradleyfalzon / gopherci

GopherCI was a project to help you maintain high-quality Go projects, by checking each GitHub Pull Request, for backward incompatible changes, and a suite of other third party static analysis tools.
https://gopherci.io
BSD 2-Clause "Simplified" License
102 stars 13 forks source link

Pull Request Reviews #115

Closed bradleyfalzon closed 7 years ago

bradleyfalzon commented 7 years ago

GitHub add PRReviewReporter to post create Pull Request Reviews

PRReviewReporter uses GitHub's Pull Request Reviews to post issues,
there are multiple benefits, the main being multiple issues result
in just one email to the user.

Additionally, when no issues are detected, a review is still created
with the event APPROVE. This will become configurable at a later
stage as it results in an extra email.

Whilst possible to set the review event to REQUEST_CHANGES, this
hasn't been enabled yet, but will become a configuration option at
a later stage. Instead, the event is set to COMMENT if there are issues
found.

Relates to #112.

Going to wait on merging this until I can get a comment on https://platform.github.community/t/api-endpoint-for-pr-reviews/409/21, hopefully, won't be long.

EDIT: This PR used to contain the addition of PRReviewReporter and enabled it by default. As we're waiting on GitHub to confirm the endpoint is available, I'll just merge adding PRReviewReporter and leave enabling it by default in another PR.

bradleyfalzon commented 7 years ago

I'm now unsure about this 

Additionally, when no issues are detected, a review is still created with the event APPROVE.

I don't think I should do this due to the extra email being sent. I think GopherCI should default to no notifications if everything is OK, and let the user enable this if they want it. Because no options are configurable at the moment, I think this should be disabled by default (setting a review to approved). This also maintains current behaviour.

bradleyfalzon commented 7 years ago

Merging leaving current APPROVE behaviour as is, it's not enabled by default yet.