-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Check .ruby-version for TargetRubyVersion #2956
Comments
PR welcome! :-) |
I'd actually prefer this not be a feature. There are a half dozen (at least) different ways that different tools try to guess ruby versions (Gemfile, gemset, |
@backus agree |
@backus so let make sure I'm following your thinking: Because we have too many ways to define the Ruby version targeted by a project, we should add another one? ;) Just because there are a lot of ways of defining the Ruby target for a project, doesn't mean that Rubocop can't do something sensible. Or define a value of The point is not to duplicate the Ruby version in 9 different places. |
This is especially important for people who have rubocop config gems or consultants who use the same rubocop config for every project. With an explicit If the |
@thegranddesign My point is that magically trying to determine the ruby version maybe saves you ten seconds of time that would have otherwise gone into dealing with a very obvious and explicit error message: If you're a contractor who really spreads himself thin across different client projects then my most generous estimate of total time lost might be a minute or two per week. Having a hidden file determine rubocop's parser is unnecessary if all it is doing is saving a few seconds for users who work on many projects. The downside is confusing users who have to track down what is going on and learn that rubocop added this feature. |
@thegranddesign Also if you really need this in your case, why don't you just add some script like this to your project? # Get ruby version
File.read('.ruby-version')
target_ruby_version = File.read('.ruby-version').to_f
# Write ruby version to rubcop config
config = YAML.load_file(".rubocop.yml")
config['AllCops'] ||= {}
config.fetch('AllCops')['TargetRubyVersion'] = target_ruby_version
# Write updated settings
File.write('.rubcop.yml', all_cops) If you use rake then you can just make this a prerequisite for the rubocop task. Otherwise you could just put this somewhere like |
Firstly, of course it's unnecessary. No one said the user had to use a Secondarily, the user put that hidden file there presumably because they wanted to lock the Ruby version. It's not like that file just magically appears one day and they're like "Gee all of a sudden my Rubocop is parsing with a different Ruby version. I wonder why." In fact, if Rubocop didn't auto-detect the Ruby version, that is the surprising behavior. Am I going to be surprised that I locked my project to Ruby 2.3 and Rubocop is using a 2.0 parser? Absolutely. Am I going to be surprised that I locked my project to 2.0 and Rubocop is complaining about optional keyword arguments? Not at all.
All of the steps you outlined are true. One could do that, but it doesn't make them any less of a hack.
I don't understand how having Rubocop auto-detect the Ruby version, or have an explicit source specified like this: AllCops:
TargetRubyVersion: 'gemfile' Which would cause an error like:
(or some other error message) is in the least bit confusing. |
Eh I disagree on your point about what is surprising behavior and what isn't. Regardless, this issue is about automatically detecting the ruby version in the absence of This is turning too much into https://xkcd.com/386/ for me so I'm going to unsubscribe from this issue. I'm just some guy who disagreed by the way, I have zero influence on rubocop. In fact it looks like @bbatsov said a PR is welcome so you could probably just implement what you want and get it merged. |
@backus I vote for relying on Personally, I find that rubocop doesn't do this surprising. And requiring separate declaration is not DRY, which itself leads to maintenance overhead. 1 minute per week * N users * M years = several lifetimes. |
@Empact I'm not aware of many other tools that use
If you think it is an obvious feature then that is fine but I don't think a lack of magic is surprising.
Heh well personally I find that a very liberal calculation of the pay off here. I will reiterate though that @bbatsov said PRs welcome so you are welcome to implement this I think. |
The only other way I'm aware of, is the Gemfile |
As originally proposed by @astrauka and properly argumented by @thegranddesign & @Empact, I think we should go ahead and (as @bbatsov suggests) submit a PR with this change (check Is someone feeling like giving it a stab? ;) |
I had this same issue and determined that rubocop wasn't giving a clear definition to the problem. It had nothing to do with my ruby version. It had to do with a mistake in my code. Once I cleared up the mistake, the rubocop warning went away. |
I keep getting this message even with defining ruby version. I guess I should look deeper into the code, who knows what rubocop complain about :) But it's not about ruby version, even if it say so... |
@alx3dev I had something similar and I found a weird "fix" to it: define an empty AllCops:
TargetRubyVersion: |
Feature: Expected behavior
Check
.ruby-version
forTargetRubyVersion
once it's not defined in.rubocop.yml
Actual behavior
Uses
1.9
as a default ofTargetRubyVersion
Steps to reproduce the problem
Do not define
TargetRubyVersion
.Use ruby named parameters
def method(param1:, param2:)
And you'll get offence
RuboCop version
The text was updated successfully, but these errors were encountered: