search

houndci / hound

Automated code review for GitHub pull requests.
https://houndci.com
MIT License
1.95k stars 401 forks source link

Github Checks API #1536

Open gareth opened 6 years ago

gareth commented 6 years ago

Hi!

We love Hound, and were wondering if there were plans to integrate it with Github's new "Checks" API (and I guess, make Hound accessible via Github Marketplace)?

salbertson commented 6 years ago

Hey @gareth, definitely! We are working on it and should be available in the Marketplace very soon.

What are you most excited about with the new Checks feature? Are you using any other services that already use Checks, like Travis? We'd love to take a look at how it's being used in the wild.

gareth commented 6 years ago

If I'm honest I don't know the ins and outs of the new API, I'm waiting to be shown what great things can be done with it. Given the impact of the change I'm assuming there are some significant benefits I haven't realised yet.

For Hound I guess not getting duplicate comments for PRs with multiple commits would be a big advantage. Obviously being able to see the linting results as their own separate result would be great. Plus any change that means I can limit the permissions I have to give integrations is a bonus.

DCzajkowski commented 6 years ago

I would also like to see a feature, where all code style violations are available through houndci.com, not via comments. With big PRs comments become very messy very quickly. Another thing is, that in current implementation (at least with eslint) Hound always passes, even though there are code violations. In my opinion it should fail, giving indication, that not all checks have succeeded.

gylaz commented 6 years ago

Another thing is, that in current implementation (at least with eslint) Hound always passes, even though there are code violations. In my opinion it should fail, giving indication, that not all checks have succeeded.

This is already possible by setting fail_on_violations: true in your .hound.yml file, as documented in our configuration.

DCzajkowski commented 6 years ago

@gylaz Awesome! Didn't know that, thanks :)

LilyReile commented 6 years ago

Big 👍 to being able to view all violations on houndci.com rather than PR comments. Ideally the "N violations found" check on GitHub would link to the appropriate page.

mmzoo commented 5 years ago

@salbertson Our use-case is simply to keep the feedback by humans separate from the feedback by the dog. If Hound inserts 10 comments in a file it's hard to see the actual code. But temporarily turning off comments, would also hide the human feedback entirely.

So this has to do with cleanness and thus work efficiency for us :) Thank you for implementing this (I hope though that this issue will not be open until 2020 or so ;)

salbertson commented 5 years ago

@mmzoo love it! Thanks for the feedback, we appreciate it.

salbertson commented 5 years ago

Is anyone interested in helping us out with this?

LilyReile commented 5 years ago

@salbertson I have Friday time at work that I could contribute. I would be interested in contributing to an option that keeps Hound feedback separate from human feedback.

LilyReile commented 5 years ago

@salbertson Back in May you mentioned that this was in progress. Anything I can work off of or would it be better to start from scratch?

salbertson commented 5 years ago

@DylanReile that work was mostly adding support for our GitHub App so we could start using Checks. Starting fresh would probably be best.

mmzoo commented 5 years ago

I just noted another disadvantage with comments: they don't disappear when they are outdated. They just hang around in the main conversation even though they have been fixed:

screen shot 2018-11-27 at 10 20 21

LilyReile commented 4 years ago

Unfortunately I bit off more than I had time to chew by volunteering for this. At the time I assumed the architecture was something like this: PR -> Hound webhook -> Run checks, assemble a big array of review comments -> batch submit comments on the PR

I further assumed I could just swap out the last batch commenting step with a batch checks uploading step. This was naive. The existing architecture actually streams comments as they become available; not all at once.

Moreover, the checks API is more sophisticated than PR -> trigger checks. There are events like pausing checks and re-running checks.

Fitting these complications into the existing control flow is more than I had time for. Sorry about that.

hlascelles commented 4 years ago

This is now needed for the built in GitHub Reminders system.

image

Without it, we can't get notifications of failures. ie, the entries in the bottom field do nothing.

Is it on the roadmap at all?