

Hound: Review JavaScript, CoffeeScript, and Ruby code for style guide violations - gkop
https://houndci.com

======
colinbartlett
I loves these sorts of robots because I hate being "that guy" who nitpicks
over things like trailing white space. (It matters!!)

There's really no way not to sound like a pedantic jerk when you comment on
someone's pull request about such things.

This at least makes the robot out to be the pedant.

~~~
tessierashpool
the other thing to do is just go through the code base later, fixing those
kinds of things. i.e., let it go on the pull request, but go through all the
code and fix every instance once a month or something.

a few years ago, IIRC, Josh Peek did that for the Rails code base and that one
commit got like five thousand "thank you" comments.

~~~
ryanto
A problem with this is it adds useless commit history. If I make a significant
change to some code, but then later on you remove some bad formatting/white
spaces the latest commit messages for that code block is most likely "removed
whitespace" and not something more meaningful.

That, and tasks that are left to be dealt with later are tasks that never get
done.

------
mck-
So I dug deeper and looked at Thoughtbot's guidelines [1] and found an epic
guideline for Javascript:

> Use CoffeeScript

[1] [https://github.com/thoughtbot/guides/blob/master/best-
practi...](https://github.com/thoughtbot/guides/blob/master/best-
practices/README.md)

------
Timothee
I like the idea behind this because I am also very sensitive to that type of
things. Probably over-sensitive but I just worry that if you let one thing go,
it becomes a slippery slope and you then have a mess of a repo.

That being said, if you care about these things, I wonder if these checks are
best left to a pre-commit hook. It removes noise from the commit history and
the PRs and forces people to think about it right away, rather than being
corrected after the fact.

I guess having that done on a server has the benefit of not having to worry
about keeping the linters' version up-to-date/homogenous over all the devs'
machines.

At work, we have JSCS ([https://github.com/jscs-dev/node-
jscs](https://github.com/jscs-dev/node-jscs)) and SCSS-lint
([https://github.com/causes/scss-lint](https://github.com/causes/scss-lint))
as part of our pre-commit hook (on top of editor plugins) and that has been
great honestly. It decreased the PR noise a lot and I feel it has been good
for new hires since it avoids having the first PR comments being about style
issues.

I wrote a post about how I added JSCS in the pre-commit hook by the way:
[http://tech.adroll.com/blog/web/2014/03/05/adding-jscs-to-
yo...](http://tech.adroll.com/blog/web/2014/03/05/adding-jscs-to-your-commit-
hook.html) It should be easily extendable to other linters.

~~~
carlio
This question comes up often when I tell people about my project Landscape
([https://landscape.io](https://landscape.io)), which is similar to this
except that it is for Python.

My argument is that often, especially on a large existing codebase, you'll get
thousands of warnings and in that case, having the trends over time is useful
as a way of measuring progress. The relative change is more important than the
absolute value.

------
jfroma
We use a linter (jshintrc) and we put a .jshintrc with the configs we agree on
our team on every project.

We work a lot with opensource projects and you need to respect the project
style. Since some of our stuff was created from other things, we can't even
keep the same jshintrc configuration for all our things.

I use SublimeLinter [1] in Sublime. It shows me the errors in an integrated
fashion.

Once the project has some maturity it is better to not change the rules
because you don't want to waste a lot of time changing stupid things like
double quotes to single quotes.

If I see 150 errors in a file, it is likely that I will introduce the 151, and
it usually means that your jshintrc is a fantasy. If I see less than 5 errors
I will fix them all and use git blame to know who introduce these bugs and
kindly recommend using an integrated linter with the editor.

I don't think is good idea to run a linter like this on any kind of CI. If
someone breaks something serious the test suite will fail anyway, and if is
not so serious why wasting time fixing it. I think is definitely better an
easier to educate your team to use some plugin on their editor.

[1]: [https://github.com/SublimeLinter/SublimeLinter-for-
ST2](https://github.com/SublimeLinter/SublimeLinter-for-ST2)

------
scotty79
Those tools could just fix style, instead of just whining about it.

I'm not even sure why code editor can produce code that does not obey agreed
upon stylistic rules. Oh, wait. It's because it's not a code editor, just a
text editor. Why don't we have cod editors yet?

~~~
tessierashpool
> Why don't we have cod editors yet?

I was going to snark that we do, they're called IDEs and they're horrible, but
then I realized something.

In between an IDE and a text editor, there could totally exist a bona fide
code editor, an in-between kind of system, but we (devs) tend to cluster our
solutions under two categories, namely text editors (raw, simple, direct) and
IDEs (complex, featureful, enormous).

This reminds me of a weird thing about testing frameworks - staggering numbers
of testing frameworks get written, yet nearly all of them tend to either use
"assert"-style syntax or "should/expect"-style syntax.

There could totally exist other families of syntax, but there don't.
(Actually, there probably do, but if so, I'm not aware of them, and it could
be because they're far from the mainstream.)

So with both these examples, you have this incredible diversity of
implementations which can be divided really easily into just two basic
categories. So even though there's an enormous amount of diversity at a very
granular level, just one conceptual level up, there's almost no diversity at
all.

So because of all this I have to say that I think your question is actually a
much better question than I initially thought.

------
jbrooksuk
I remember when Hound first launched and every time somebody commented on the
same line of code, Hound would persistently comment back with the same
warning. It was amusing!

I'm currently working on a PHP version of Hound, called Anorak[1]. At the
moment I'm part of a small team building a PHP replacement of Rubocop called
Hippo[2]. We originally wrote a basic tokenizer with regex, but although it
works well, we realise it's slow, so we're rebuilding it with "token_get_all"
and hacking around that.

[1] [http://anorakci.com](http://anorakci.com)

[2] [https://github.com/hippophp/hippo](https://github.com/hippophp/hippo)

------
doomspork
Really helpful and useful tool but we ultimately had to quit using it because
of the noise. Rather than a comment per violation I'd like to see an approach
similar to CodeClimate's: one comment and a link to a more detailed report.

------
ahuth
It's interesting that Hound itself is a Rails app [1]. And it seems to be
really well organized / put together. Thoughtbot is definitely following Sandi
Metz's rules for developers [2].

There are a lot of small, plain Ruby classes that are very easy to understand.
Must be relatively easy to maintain and add new features.

[1] [https://github.com/thoughtbot/hound](https://github.com/thoughtbot/hound)

[2] [http://robots.thoughtbot.com/sandi-metz-rules-for-
developers](http://robots.thoughtbot.com/sandi-metz-rules-for-developers)

------
eltaco
I've been using jscs [1] for js style checks.

[1] [https://github.com/jscs-dev/node-jscs](https://github.com/jscs-dev/node-
jscs)

~~~
MAGZine
JSCS is quite good. Also, I use the jsdoc plugin to validate and make sure the
jsdoc comments are up-to date.

------
simonsarris
For people whinging about "why doesn't it just fix the small stuff", I've been
using Google's Closure Linter for years to fix JS style with the `fixjsstyle`
command. It's grand for that purpose:

[https://developers.google.com/closure/utilities/docs/linter_...](https://developers.google.com/closure/utilities/docs/linter_howto)

------
andreasklinger
We use linters as part of our CI process. So actually "tests" fail if code is
not within standards of rubocop or coffee lint.

Personally I prefer this over cluttering the PR process.

Codelinters are imho less about "style" than about best practices. And many
exist for good reason other just for uniformity. Both is usually important
enough to enforce it.

~~~
gfodor
The point of contention on this is if you are doing something approximating
continuous deployment. In that scenario, the tests and build are part of
operations, since they are prerequisites to being able to ship code. As such,
in an emergency scenario, you do not want to be in a situation where time to
recovery is impacted by someone committing a hotfix that has improper
whitespace, etc. In the best case, you end up spending additional time having
to to 'sign off' on the broken build being just due to the linter (which also
introduces a real risk in a high stakes scenario, in that you are shipping a
red build and now have another place for human error to creep in) or in the
worst case, the system blocks deploys until a commit is pushed to undo the
whitespace problem.

~~~
andreasklinger
1) real emergencies should happen very rarely

2) if they do you dont want to wait until the tests pass - so it's outside of
that process anyhow

3) the real root problem here is: "emergency deploys need to be fast and
adhoc" \- not "linting is part of the ci process"

~~~
gfodor
I'm not sure I agree -- having an alternate process for deployments in
emergency scenarios is just asking for all kinds of trouble. It should be a
well oiled machine that is consistent. If running the tests is part of the
process, it should always be part of the process, otherwise you introduce
additional complexity in trying to understand the potential outcomes of the
system in unique scenarios. If the tests are slow that they gunk up the works,
then the tests need to be fixed and the process changed. (Perhaps a large
suite which runs nightly over less critical code, and a tight suite for
mission critical code that runs pre-deploy.)

------
iamdanfox
This looks pretty cool as a super easy to configure CI! I'm sure big projects
will stick to Travis, but if you haven't got your linter set up yet this looks
ideal.

I wonder how hard it would be for the bot to autofix stuff too? Perhaps it
could serve a git remote and allow you to pull all the fixes in one go?

------
CWIZO
Shame this only works with GitHub. We use stash at work. And I would LOVE to
have this. I'm very sensitive to this stuff, and I always have to be that guy
:/

