14 ms·

Hound: A service that comments on Ruby style violations in GitHub pull requests

69 points by wymy 12y ago

spajus 12y ago
Take a look at https://github.com/mmozuras/pronto https://github.com/mmozuras/pronto - does the same, has many adapters.
- russianbandit 12y ago
  Hounds looks easier than pronto. Just turn it on for a repo and it does it's thing. With pronto looks like I have to configure it, then run commands (could make it part of default rake task, is suppose).
  - spajus 12y ago
    Yes, we run pronto when our Jenkins builds GitHub pull requests, so we get all those comments along with peer code reviews. And no separate web app is necessary.
deleted 12y ago
[deleted]
guptaneil 12y ago
This is really cool. Sure, similar tools have existed for a while, but Hound looks far simpler to setup and use than previous tools. However, it would be nice to have this optionally run on every commit rather than only on pull requests. For my own personal projects, I obviously don't submit pull requests to myself, but I would like to have Hound double-check my code for me.
- dmur 12y ago
  Not sure it should be obvious you don't. Pull requests are especially well-suited for a team environment, but using PRs in a solo project can still give a higher level of structure to the progression of your code. Gives you an opportunity to explain why you're doing what you're doing.
  - caleb_thompson 12y ago
    Agreed. This approach is especially nice for situations where you open up the code at a later point, so that people can see the historic progression in more ways than just the Git history. https://github.com/thoughtbot/hound/pull/1 https://github.com/thoughtbot/hound/pull/1
  - sanderjd 12y ago
    You can also accomplish this without pull requests by writing good messages in your merge commits like you would in a pull request description.
mikesilvis 12y ago
https://github.com/bbatsov/rubocop https://github.com/bbatsov/rubocop FTW. Integrate it into your test suite and you can see your mistakes before you push up a change for your team to see.
- huntedsnark 12y ago
  There is also a guard plugin for immediate feedback: https://github.com/yujinakayama/guard-rubocop https://github.com/yujinakayama/guard-rubocop Rubocop can even correct some style errors automatically now!
  - llamataboot 12y ago
    Hound is built on Rubocop
- Croaky 12y ago
  Rubocop is great! Hound is built on top of it. We've tried putting style violations in the build and found there are enough edge cases that it isn't exactly the interaction we've wanted. Sometimes, we want the human to say "no, my pair and I broke the guideline on purpose and we're okay with it in this case." We don't want a broken build in those cases. Totally agree with you about getting feedback earlier than opening a PR, though. Linters integrated into text editors are a great way to go.
caleb_thompson 12y ago
One thing I've liked with hound is that when it comes up with what I'd consider a "false positive," such as a line with a url that can't be split, I can just reply inline to explain why that couldn't change. This pull has a lot of examples of that: https://github.com/thoughtbot/griddler/pull/119 https://github.com/thoughtbot/griddler/pull/119
- TazeTSchnitzel 12y ago
  It's hilarious how houndci acts like some stubborn human, responding to complaints with the exact same suggestion.
  - caleb_thompson 12y ago
    Yeah, that happens if you force push.
    - salbertson 12y ago
      That's not good. I opened an issue on GitHub. https://github.com/thoughtbot/hound/issues/182 https://github.com/thoughtbot/hound/issues/182
strzalek 12y ago
I hate those github bots. Just use rubocop and run it in your CI.
- salbertson 12y ago
  Hound is opt-in, it won't ever add itself to your repo and start commenting.
jared314 12y ago
I have always liked the idea of these bots roaming github, looking for bugs to fix and code that could be improved. But, it almost always ends up annoying somehow. Either the change is a false positive, the bot doesn't understand the project structure, or the bot just spams changes. Software development has too many unwritten rules and social interactions for a bot to just send PRs and comments. Perhaps another type of communication, other than issues, PRs, and comments, is needed just for bots. I would tolerate my projects being scanned, if I could separate the human and bot communication streams. (And, block bots when they don't work as expected.)
- Croaky 12y ago
  You can tune Hound (and Rubocop) to your liking: https://houndci.com/configuration https://houndci.com/configuration > the change is a false positive I've been using Hound on about a dozen projects the last few months and haven't seen false positives. We built it atop Rubocop, which has been pretty well-vetted. > Software development has too many unwritten rules and social interactions for a bot We've limited our guidelines to a subset that should almost always be "no argument, my bad, fixed in [SHA hash]." However, it comments instead of failing the build or mechanically changing the code because that the human should make the final decision about whether to make the change.
rubiquity 12y ago
I love this. On just about any team I have worked on, whenever a fresh PR comes in everyone goes through it picking out style guide violations. It's tedious, annoying and sometimes arbitrary if the team doesn't have a solid style guide in place. Hound will let us get right to reviewing what matters: the code itself. Up next: I would love a Unix utility I could filter my uncommitted diff through and get style guide violations before I even upload to GitHub. Use git for everything, baby.
- jwinter 12y ago
  This exists and is what Hound is built on. Rubocop runs lint checks against your Ruby code: https://github.com/bbatsov/rubocop/ https://github.com/bbatsov/rubocop/ . The Emacs integration is great; it highlights failures in place. Hound looks interesting, but having Emacs/Vim integration gives you a tighter feedback loop, so you don't have to wait until Pull Request time to discover these issues.
  - gknoy 12y ago
    You can also use linting tools as a build validation step (ours are invoked via things like `make myapp.pep8test`). These can also then be invoked yourself before making a commit. As jwinter says, this really tightens the loop when fixing trivial errors. Almost all of the tools are very configurable in terms of which warnings/errors you want to ignore (e.g., let our max line length be 120 instead of 80), and give pretty good feedback (line numbers, exact error). Another great benefit is that you have a neutral arbiter of what's the style for your codebase, which helps avoid ruffled feathers.
  - rubiquity 12y ago
    I figured such a tool must exist. Thank you for helping me remain lazy. I agree about having the tighter feedback loop. I'm hoping to create a git filter that stops the commit from happening if there are style guide violations.
  - salbertson 12y ago
    RuboCop is great, but one benefit of Hound is it only checks code that has changed. Retroactively enforcing code style is usually a bad idea.
paultannenbaum 12y ago
This is great. Is it possible to use this as a rake task? I think it would be better to have team members run this locally just like they would run a test suite, make sure everything is up to par, and then open a PR. This would keep the communication stream in the PR nice and clean, focused only on code implementation.
- mikesilvis 12y ago
  https://github.com/bbatsov/rubocop https://github.com/bbatsov/rubocop
jeez444 12y ago
This is a great product. I was working on a product when a new developer joined and started writing Ruby like he was writing in PHP, ignoring all of the programming language's idioms. What was worse was that with every commit where his code was "cleaned up", he'd put them back in!
- salbertson 12y ago
  Glad you like it!
l_ew_is 12y ago
Not sure I'd want to give this site Github access: http://filippo.io/Heartbleed/#houndci.com http://filippo.io/Heartbleed/#houndci.com
- russianbandit 12y ago
  We re-signed our certificates after Heroku patched the issue.
- salbertson 12y ago
  Read the FAQ, this is expected when the server has been patched as long as the site is responding.
  - l_ew_is 12y ago
    It was giving me a "houndci.com IS VULNERABLE." 45 mins ago. http://l.ew.is/shared/houndci.com_IS_VULNERABLE.png http://l.ew.is/shared/houndci.com_IS_VULNERABLE.png
    - salbertson 12y ago
      Is that still the case? I'm seeing a timeout when I run the test. https://www.dropbox.com/s/txlzh5jzrt30afs/Screenshot%202014-04-09%2015.13.37.png https://www.dropbox.com/s/txlzh5jzrt30afs/Screenshot%202014-... Again, we have reissued our certificates and installed them since the servers were patched.
VeejayRampay 12y ago
Can someone shed some light on the reason why we're still bothering with "line is too long" in 2014? I mean I'm not talking 300 characters-long lines here that's ridiculous, but the idea that a line with 90 characters is "too long" because some arbitrary limit in the width of certain terminal emulators back in the days was fixed at whatever number of columns seems really backwards. So yeah, I'm open too any sane explanation of the why that is actually good practice beyond what I perceive as severe cargo culting.
- russianbandit 12y ago
  A lot of developers use vim, with split buffers. When code lines wrap, they look ugly.
  - psychometry 12y ago
    But other text editors handle long lines perfectly well. I don't think other developers' choice of tools should dictate coding styles. I exclude the tab/spaces debate from this because it has ramifications outside of mere stylistic concerns.
    - scott_karana 12y ago
      There are only two possible ways to deal with long lines extending past the edge of the window. 1. Scroll off the side of the window Pain in the ass. Ever try diffing a file, and had to scroll? 2. Wrap Also a pain in the ass. Formatting alignment, confusion over where important linebreaks are, etc.
      - roryokane 12y ago
        3. Wrap, and use a good editor Many editors don’t fully support soft wrapping, sadly. But if you’re lucky enough to use an editor that includes the necessary features, there are no problems with soft wrapping. Soft wrapping has the advantage that you never have to manually re-wrap a line. These editor features make soft wrapping useable: Display noneditable symbols at wrap points to mark them as nonimportant linebreaks. In Vim, `set showbreak=->` to display “->” at wrapped lines. Emacs supports putting arrow symbols in the “fringe”. Visual Studio has a preference to enable this. Indent wrapped lines at the same indentation as the start of the line, optionally plus extra indentation. Vim does not support this but there is a patch for Vim that makes this enable-able with `set breakindent`. Coda supports it fully, with customizable extra indentation. This is the least-supported, most-important feature of making wrapping useable. Separate logical-line movement and visual-line movement. Pretty much every editor supports this. Logical-line movement is really only necessary in macros, anyway. In Vim, it’s j/k (or -/<Enter>) vs gj/gk. In Sublime Text, it’s up/down vs option-up/option-down (or some combination like that).
100k 12y ago
The marketing site needs some work. There's no details about pricing that I can find and no information about configuring your own rules (I had to read the source code to see if that was possible). I am not going to sign up with GitHub to see either of those things, they need to be public.
- russianbandit 12y ago
  It's a free service right now. We'll be taking all the suggestions and improving it as we go.
  - 100k 12y ago
    The announcement blog post implies otherwise: > It places the focus value not on our super-secret-source-code but on our handling of the hosting, billing, and maintenance of that source code.
    - salbertson 12y ago
      Sorry if it wasn't clear in the post but we do plan on charging for Hound in some way. "We intend to charge in the near future to ensure Hound is sustainable, but public projects will continue to be free."
      - 100k 12y ago
        I don't expect this to be free. My point is that pricing should be clearly stated on the landing page for the product. You should have a pricing link or header, even if it just says "free for now, we're working on pricing".
        salbertson 12y ago
        Got it, thanks for the input.

69 points by wymy 12y ago

Take a look at https://github.com/mmozuras/pronto https://github.com/mmozuras/pronto - does the same, has many adapters.

russianbandit 12y ago
Hounds looks easier than pronto. Just turn it on for a repo and it does it's thing. With pronto looks like I have to configure it, then run commands (could make it part of default rake task, is suppose).
- spajus 12y ago
  Yes, we run pronto when our Jenkins builds GitHub pull requests, so we get all those comments along with peer code reviews. And no separate web app is necessary.

deleted 12y ago

[deleted]

guptaneil 12y ago

This is really cool. Sure, similar tools have existed for a while, but Hound looks far simpler to setup and use than previous tools. However, it would be nice to have this optionally run on every commit rather than only on pull requests. For my own personal projects, I obviously don't submit pull requests to myself, but I would like to have Hound double-check my code for me.

dmur 12y ago
Not sure it should be obvious you don't. Pull requests are especially well-suited for a team environment, but using PRs in a solo project can still give a higher level of structure to the progression of your code. Gives you an opportunity to explain why you're doing what you're doing.
- caleb_thompson 12y ago
  Agreed. This approach is especially nice for situations where you open up the code at a later point, so that people can see the historic progression in more ways than just the Git history. https://github.com/thoughtbot/hound/pull/1 https://github.com/thoughtbot/hound/pull/1
- sanderjd 12y ago
  You can also accomplish this without pull requests by writing good messages in your merge commits like you would in a pull request description.

mikesilvis 12y ago

https://github.com/bbatsov/rubocop https://github.com/bbatsov/rubocop FTW. Integrate it into your test suite and you can see your mistakes before you push up a change for your team to see.

huntedsnark 12y ago
There is also a guard plugin for immediate feedback: https://github.com/yujinakayama/guard-rubocop https://github.com/yujinakayama/guard-rubocop Rubocop can even correct some style errors automatically now!
- llamataboot 12y ago
  Hound is built on Rubocop
Croaky 12y ago
Rubocop is great! Hound is built on top of it. We've tried putting style violations in the build and found there are enough edge cases that it isn't exactly the interaction we've wanted. Sometimes, we want the human to say "no, my pair and I broke the guideline on purpose and we're okay with it in this case." We don't want a broken build in those cases. Totally agree with you about getting feedback earlier than opening a PR, though. Linters integrated into text editors are a great way to go.

caleb_thompson 12y ago

One thing I've liked with hound is that when it comes up with what I'd consider a "false positive," such as a line with a url that can't be split, I can just reply inline to explain why that couldn't change. This pull has a lot of examples of that: https://github.com/thoughtbot/griddler/pull/119 https://github.com/thoughtbot/griddler/pull/119

TazeTSchnitzel 12y ago
It's hilarious how houndci acts like some stubborn human, responding to complaints with the exact same suggestion.
- caleb_thompson 12y ago
  Yeah, that happens if you force push.
  - salbertson 12y ago
    That's not good. I opened an issue on GitHub. https://github.com/thoughtbot/hound/issues/182 https://github.com/thoughtbot/hound/issues/182

strzalek 12y ago

I hate those github bots. Just use rubocop and run it in your CI.

salbertson 12y ago
Hound is opt-in, it won't ever add itself to your repo and start commenting.

jared314 12y ago

I have always liked the idea of these bots roaming github, looking for bugs to fix and code that could be improved. But, it almost always ends up annoying somehow. Either the change is a false positive, the bot doesn't understand the project structure, or the bot just spams changes. Software development has too many unwritten rules and social interactions for a bot to just send PRs and comments. Perhaps another type of communication, other than issues, PRs, and comments, is needed just for bots. I would tolerate my projects being scanned, if I could separate the human and bot communication streams. (And, block bots when they don't work as expected.)

Croaky 12y ago
You can tune Hound (and Rubocop) to your liking: https://houndci.com/configuration https://houndci.com/configuration > the change is a false positive I've been using Hound on about a dozen projects the last few months and haven't seen false positives. We built it atop Rubocop, which has been pretty well-vetted. > Software development has too many unwritten rules and social interactions for a bot We've limited our guidelines to a subset that should almost always be "no argument, my bad, fixed in [SHA hash]." However, it comments instead of failing the build or mechanically changing the code because that the human should make the final decision about whether to make the change.

rubiquity 12y ago

I love this. On just about any team I have worked on, whenever a fresh PR comes in everyone goes through it picking out style guide violations. It's tedious, annoying and sometimes arbitrary if the team doesn't have a solid style guide in place. Hound will let us get right to reviewing what matters: the code itself. Up next: I would love a Unix utility I could filter my uncommitted diff through and get style guide violations before I even upload to GitHub. Use git for everything, baby.

jwinter 12y ago
This exists and is what Hound is built on. Rubocop runs lint checks against your Ruby code: https://github.com/bbatsov/rubocop/ https://github.com/bbatsov/rubocop/ . The Emacs integration is great; it highlights failures in place. Hound looks interesting, but having Emacs/Vim integration gives you a tighter feedback loop, so you don't have to wait until Pull Request time to discover these issues.
- gknoy 12y ago
  You can also use linting tools as a build validation step (ours are invoked via things like `make myapp.pep8test`). These can also then be invoked yourself before making a commit. As jwinter says, this really tightens the loop when fixing trivial errors. Almost all of the tools are very configurable in terms of which warnings/errors you want to ignore (e.g., let our max line length be 120 instead of 80), and give pretty good feedback (line numbers, exact error). Another great benefit is that you have a neutral arbiter of what's the style for your codebase, which helps avoid ruffled feathers.
- rubiquity 12y ago
  I figured such a tool must exist. Thank you for helping me remain lazy. I agree about having the tighter feedback loop. I'm hoping to create a git filter that stops the commit from happening if there are style guide violations.
- salbertson 12y ago
  RuboCop is great, but one benefit of Hound is it only checks code that has changed. Retroactively enforcing code style is usually a bad idea.

paultannenbaum 12y ago

This is great. Is it possible to use this as a rake task? I think it would be better to have team members run this locally just like they would run a test suite, make sure everything is up to par, and then open a PR. This would keep the communication stream in the PR nice and clean, focused only on code implementation.

mikesilvis 12y ago
https://github.com/bbatsov/rubocop https://github.com/bbatsov/rubocop

jeez444 12y ago

This is a great product. I was working on a product when a new developer joined and started writing Ruby like he was writing in PHP, ignoring all of the programming language's idioms. What was worse was that with every commit where his code was "cleaned up", he'd put them back in!

salbertson 12y ago
Glad you like it!

l_ew_is 12y ago

Not sure I'd want to give this site Github access: http://filippo.io/Heartbleed/#houndci.com http://filippo.io/Heartbleed/#houndci.com

russianbandit 12y ago
We re-signed our certificates after Heroku patched the issue.
salbertson 12y ago
Read the FAQ, this is expected when the server has been patched as long as the site is responding.
- l_ew_is 12y ago
  It was giving me a "houndci.com IS VULNERABLE." 45 mins ago. http://l.ew.is/shared/houndci.com_IS_VULNERABLE.png http://l.ew.is/shared/houndci.com_IS_VULNERABLE.png
  - salbertson 12y ago
    Is that still the case? I'm seeing a timeout when I run the test. https://www.dropbox.com/s/txlzh5jzrt30afs/Screenshot%202014-04-09%2015.13.37.png https://www.dropbox.com/s/txlzh5jzrt30afs/Screenshot%202014-... Again, we have reissued our certificates and installed them since the servers were patched.

VeejayRampay 12y ago

Can someone shed some light on the reason why we're still bothering with "line is too long" in 2014? I mean I'm not talking 300 characters-long lines here that's ridiculous, but the idea that a line with 90 characters is "too long" because some arbitrary limit in the width of certain terminal emulators back in the days was fixed at whatever number of columns seems really backwards. So yeah, I'm open too any sane explanation of the why that is actually good practice beyond what I perceive as severe cargo culting.

russianbandit 12y ago
A lot of developers use vim, with split buffers. When code lines wrap, they look ugly.
- psychometry 12y ago
  But other text editors handle long lines perfectly well. I don't think other developers' choice of tools should dictate coding styles. I exclude the tab/spaces debate from this because it has ramifications outside of mere stylistic concerns.
  - scott_karana 12y ago
    There are only two possible ways to deal with long lines extending past the edge of the window. 1. Scroll off the side of the window Pain in the ass. Ever try diffing a file, and had to scroll? 2. Wrap Also a pain in the ass. Formatting alignment, confusion over where important linebreaks are, etc.
    - roryokane 12y ago
      3. Wrap, and use a good editor Many editors don’t fully support soft wrapping, sadly. But if you’re lucky enough to use an editor that includes the necessary features, there are no problems with soft wrapping. Soft wrapping has the advantage that you never have to manually re-wrap a line. These editor features make soft wrapping useable: Display noneditable symbols at wrap points to mark them as nonimportant linebreaks. In Vim, `set showbreak=->` to display “->” at wrapped lines. Emacs supports putting arrow symbols in the “fringe”. Visual Studio has a preference to enable this. Indent wrapped lines at the same indentation as the start of the line, optionally plus extra indentation. Vim does not support this but there is a patch for Vim that makes this enable-able with `set breakindent`. Coda supports it fully, with customizable extra indentation. This is the least-supported, most-important feature of making wrapping useable. Separate logical-line movement and visual-line movement. Pretty much every editor supports this. Logical-line movement is really only necessary in macros, anyway. In Vim, it’s j/k (or -/<Enter>) vs gj/gk. In Sublime Text, it’s up/down vs option-up/option-down (or some combination like that).

100k 12y ago

The marketing site needs some work. There's no details about pricing that I can find and no information about configuring your own rules (I had to read the source code to see if that was possible). I am not going to sign up with GitHub to see either of those things, they need to be public.

russianbandit 12y ago
It's a free service right now. We'll be taking all the suggestions and improving it as we go.
- 100k 12y ago
  The announcement blog post implies otherwise: > It places the focus value not on our super-secret-source-code but on our handling of the hosting, billing, and maintenance of that source code.
  - salbertson 12y ago
    Sorry if it wasn't clear in the post but we do plan on charging for Hound in some way. "We intend to charge in the near future to ensure Hound is sustainable, but public projects will continue to be free."
    - 100k 12y ago
      I don't expect this to be free. My point is that pricing should be clearly stated on the landing page for the product. You should have a pricing link or header, even if it just says "free for now, we're working on pricing".
      - salbertson 12y ago
        Got it, thanks for the input.