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 to not approve PR by default #123

Closed bradleyfalzon closed 7 years ago

bradleyfalzon commented 7 years ago

Since implementing Pull Request Reviews in #115 and #116, additional changes were made to approve a pull request. But this causes some additional behaviour that might be undesirable, such as emailing the user that everything is OK and having extra UI elements on PR showing it's been reviewed.

I don't mind if people want to enable this, but I don't think it provides value by default. I'll disable this part by default for the moment.

bradleyfalzon commented 7 years ago

@shurcooL, I saw you gave https://github.com/bradleyfalzon/gopherci/pull/115#issuecomment-322029712 a 👍 - do you agree?

dmitshur commented 7 years ago

I agree.

In fact, the specific part of that comment I was giving a 👍 to was this sentence:

I think GopherCI should default to no notifications if everything is OK, and let the user enable this if they want it.

It seems to be consistent with go vet output, etc. If there are no issues, it's silent. I like that.

dmitshur commented 7 years ago

That said, I think there's a good chance that further evidence may suggest a different behavior is better. Iterating on this is completely expected and okay.

bradleyfalzon commented 7 years ago

Thanks @shurcooL, I'll make the change but eventually users can chose their desired behaviour.