Fixes #34 - Adding hound and rubocop config and coveralls gem #39
Conversation
|
Changes Unknown when pulling 37e2fbbdfeed2dc49167aac76bea3a555e27d686 on feature/code_review_tool into ** on develop**. |
hweng
force-pushed
the
feature/code_review_tool
branch
from
37e2fbb to
c1a29c5
Compare
|
Changes Unknown when pulling c1a29c5957cc966760bd4b181fd7eb2c625acf43 on feature/code_review_tool into ** on develop**. |
|
@ucsdlib/developers Please review, comments ... Thanks. |
| jshint: | ||
| enabled: false | ||
|
|
||
| fail_on_violations: true |
There was a problem hiding this comment.
👍 is everyone else good with failing on violations?
.rubocop.yml
Outdated
| - 'app/**/*' | ||
|
|
||
| Style/Next: | ||
| Enabled: false |
There was a problem hiding this comment.
Use next to skip iteration instead of a condition? It is fine to me if you want to stick to that.
There was a problem hiding this comment.
I was just wondering why it was selectively disabled, since it is a default (and default syntax convention in ruby). I would vote for having it enabled, but it's up to you all of course. @lsitu and @VivianChu any preference? To give context, we're talking about the following:
http://rubocop.readthedocs.io/en/latest/cops_style/#stylenext
There was a problem hiding this comment.
@mcritchlow Yeah, here are some comments from other folks including bbatsov about this cop: rubocop/rubocop#1238
There was a problem hiding this comment.
It's fine to have it enabled. I got it enabled for dmr and use next to skip iteration.
There was a problem hiding this comment.
@hweng yeah, there are some examples in that issue I could see being annoying to have described as a "failure", since some of those are subjectively better syntax than using next.
Generally it seems like good practice to use next as a guard clause, though if it got annoying or we had use cases like the ones mentioned crop up, we could always disable again or experiment with the MinBodyLength setting? Just a thought.
There was a problem hiding this comment.
@mcritchlow Sounds fine to me, I will enable it now as you all voted it.
Just provide some of other folk's experience so that we could realize both sides of it.
And as you mentioned, we could always disable it if it is getting annoying.
hweng
force-pushed
the
feature/code_review_tool
branch
from
c1a29c5 to
4da6503
Compare
|
@ucsdlib/developers The style/next cop is enabled. Please review and merge it after the CircleCI tests passed. Thanks! |
|
Changes Unknown when pulling 4da6503 on feature/code_review_tool into ** on develop**. |
|
👍 |
No description provided.