varvet / pundit

Minimal authorization through OO design and pure Ruby classes
MIT License
8.22k stars 625 forks source link

Update rubocop #708

Closed MarceloAGuimaraes closed 2 years ago

Burgestrand commented 2 years ago

Nice!

The JRuby error is peculiar:

Resolving dependencies...
Bundler found conflicting requirements for the Ruby version:

  In Gemfile:
    Ruby (~> 2.5.8.0)

    pundit was resolved to 2.1.1, which depends on
      Ruby (>= 2.6.0)

I'm not sure where it's getting the idea that pundit is specifically depending on Ruby 2.6. AFAIK there's no required_ruby_version in our gemspec. Maybe it's because of that bug mentioned in Travis.

Burgestrand commented 2 years ago

Ah, actually, never mind I missed this: https://github.com/varvet/pundit/pull/708/files#diff-4821dcfe039387422204641478a2e53aa114df34dc8fec77a5df47effdc9e2d0R21

That's probably the cause of the JRuby failure 😄

MarceloAGuimaraes commented 2 years ago

I added a required_ruby_version to solve a log from rubocop, but i forgot about JRuby. I just removed the line from gemspec and disabled the rule(Gemspec/RequiredRubyVersion) that was requiring it.

Burgestrand commented 2 years ago

I added a required_ruby_version to solve a log from rubocop, but i forgot about JRuby. I just removed the line from gemspec and disabled the rule(Gemspec/RequiredRubyVersion) that was requiring it.

Nice work! Could you please update the commit message of https://github.com/varvet/pundit/pull/708/commits/08e23a1c25f3695c7339c7653e14249520a0c164 so that it explains why it was removed? It's to help our future friend that comes along and wants to know why we've chosen to disable it.

An example would be:

JRuby jruby-9.2 considers itself to be compliant with MRI 2.5, so adding a minimum required ruby version of 2.6 causes bundler to fail with the following error:

Resolving dependencies...
Bundler found conflicting requirements for the Ruby version:

 In Gemfile:
   Ruby (~> 2.5.8.0)

   pundit was resolved to 2.1.1, which depends on
     Ruby (>= 2.6.0)
MarceloAGuimaraes commented 2 years ago

@Burgestrand makes totally sense. I just updated the commit message.

Burgestrand commented 2 years ago

Awesome, thank you, well done!