-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update Rubocop rules #3872
Comments
In my view, maintaining a set of Rubocop rules is more trouble than it is worth when we could use standard that allows us to avoid style arguments, avoid maintaining our own configs, and just use the default settings. I'm very much not a fan of Rubocop's defaults and it's not like standard has made all the same choices I would have, but I'm happy to go along with their choices, especially given the thoughtfulness they've put into each of them and how they've responded to community feedback and backed out of a couple controversial changes. |
I agree, it's probably better to switch to standard and stop worrying about coding style altogether rather than trying to come up with our own rules for every situation. As for Hound, I think we should stop using it. Because of its architecture, it has a number of problems (mainly that you can't use any RuboCop plug-ins it doesn't support, and the way it handles comments on PRs) and there are better solutions out there nowadays. Even just running RuboCop as part of our CI would be a better (and much simpler) solution imo. |
I added
Including things like Does it makes sense to make all these changes? |
Yeah, our current Rubocop configuration doesn't line up well with Standard's preferences. I'm not sure a wholesale update is feasible, but we're going to have to do something here. Do you happen to know what percentage of those offenses are just quotes? We can configure Standard to prefer single quotes and potentially make a couple other compromises if it make this update less prohibitive. |
@jarednorman I added standard, disabled a few rubocop rules and fixed some warnings. #3951 For some reason I am getting a bunch of hound-ci warnings and CI failures. |
I explained over here that we're going to just stick with Rubocop for now. We can move forward with auditing the rules we're using and updating them. |
We have quite an old set of Rubocop rules. We should evaluate the impact of updating the rules we have to a new set. Also, we should consider that at the moment Hound is performing the checks and we should be compatible with one of the versions they support.
The text was updated successfully, but these errors were encountered: