Skip to content
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

[Fix #8761] Read required_ruby_version from gemspec file if it exists #8885

Merged

Conversation

HeroProtagonist
Copy link
Contributor

@HeroProtagonist HeroProtagonist commented Oct 10, 2020

Closes #8761

  • Added GemspecFile class to search upwards for <project_base>.gemspec and read from required_ruby_version if present in the file
  • Placed it in the Sources array right before Default
  • Created specs over new class
  • Update docs to clarify which files are looked at for ruby version
  • Updated changelog

Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@pBorak
Copy link

pBorak commented Oct 13, 2020

@HeroProtagonist
Hey, great job, I was also doing some work related to this issue :)
One question though, have you taken under consideration that the ruby version format in Gemspec may be written in different ways?
s.required_ruby_version = '< 3.0.0' or even s.required_ruby_version =['>= 2.2.0', '< 2.3.0']

@HeroProtagonist
Copy link
Contributor Author

@pBorak Right now mine is pretty simple, just checking for the numbers after required_ruby_version.

The other files where version are checked are here:
https://github.com/rubocop-hq/rubocop/blob/de5b74fece1166133cedd860c07c75d5a4426cbc/lib/rubocop/target_ruby.rb#L133

I guess the question to be answered is how they support those cases, where applicable, and then matching that behavior

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 4, 2020

I'm fine with the proposed change, but we should document the new behavior as otherwise it might confuse people.

I guess the question to be answered is how they support those cases, where applicable, and then matching that behavior

As for bounded version - I guess you can check for an array and extract a minimum version from there. If only a maximum version is present there's nothing for us to do.

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 7, 2020

You'll also need to rebase on top of master.

@HeroProtagonist
Copy link
Contributor Author

@bbatsov updated and rebased 👍

@bbatsov bbatsov merged commit 406d0e3 into rubocop:master Nov 8, 2020
@bbatsov
Copy link
Collaborator

bbatsov commented Nov 8, 2020

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read TargetRubyVersion from gemspec required_ruby_version if not specified
3 participants