-
Notifications
You must be signed in to change notification settings - Fork 463
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
Get the build green for modern ruby and rack and rubocop-rspec #939
Get the build green for modern ruby and rack and rubocop-rspec #939
Conversation
As part of rack/rack#2137 (released in Rack 3) Rack changed the values of `Rack::Utils::SYMBOL_TO_STATUS_CODE` so asking for `:unprocessable_entity` now returns `nil`. The guidance has always been to use `Rack::Util.status_code` (present in the API since Rack 1.1) so let's just do that.
Ever since rack/rack#2018 `rack.input` has been optional so we can't assume it's there and call `read` or `rewind` on it in `Apipie::Extractor::Recorder`. Using safe navigation to call these methods seems like the simplest approach because we want to look for `rack.request.form_hash` instead in both the situation where `rack.input` is missing, or it's empty.
In rubocop/rubocop-rspec#1848 rubocop-rspec extracted some cops to separate gems, and because we referenced one of them in our rubocop_todo.yml the whole rubocop process wouldn't run because it doesn't like config for cops it doesn't know about. We introducing the missing gem, `rubocop-rspec_rails`, and fix the name in the config to let us run rubocop again. There is one infraction that exists in the source so we fix that to get us to a green rubocop run.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏽 LGTM
apipie-rails.gemspec
Outdated
@@ -38,6 +38,7 @@ Gem::Specification.new do |s| | |||
s.add_development_dependency 'rubocop-rails' | |||
s.add_development_dependency 'rubocop-rspec' | |||
s.add_development_dependency 'rubocop-performance' | |||
s.add_development_dependency 'rubocop-rspec_rails' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you try adding if RUBY_VERSION >= "2.7"
please?
or take out this line for now, ... if you can
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do that, but the problem will probably then be that ruby 2.6 rubocop will complain about RSpecRails/InferredSpecType
being a cop it doesn't understand whereas ruby 2.7+ rubocop complains about RSpec/Rails/InferredSpecType
. Can we put conditional rubocop_todo by ruby version?
In another thread I think you suggested someone had dropped ruby 2.6 / rails 5 support - should we make that official?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll push up a commit with the if
version comparison so we can test that theory though
It's only available for 2.7+ and we still run this build on 2.6.
@h-lame The condition is cool, we just need to add a rubocop exclusion for it, I think its fine, I will probably remove support for ruby 2.6 soon anyway, not much of a point anymore. bit in the mean time. its good. |
Thanks! Reading up on the now failing rubocop complaint about using RUBY_VERSION in the gem spec, sounds like the preferred option is moving all the |
@h-lame No thanks, the condition is for ruby 2.6, which we'll probably remove support for. so that exclusion will go away anyway soon. about the comment you linked on rubygems, I don't see how rubygems will be able to deprecate thanks for your contribution to Apipie |
Yeah, I agree, seems like a pretty big breaking change to leave in a comment on an unrelated issue.
No worries! Thanks for accepting it. |
Separated out from #938 this PR attempts to get the master build to green from a fresh checkout.
These days you're probably on ruby 3.x and will get rack 3.x and rubocop-rspec 3.x too. These new versions all cause problems with the code in master, so the build won't pass.
Rack::Utils::SYMBOL_TO_STATUS_CODE
are now different and this causes the dummy rails app we use for testing to not load and this blocks the entire suite from running. The solution is to useRack::Utils.status_code
which has a workaround for the old names.env['rack.input']
optional. rack/rack#2018 - Handle the fact thatrack.input
is now optional in the rack env. The solution is to use safe navigation to call the methods we wanted.Capybara
,FactoryBot
andRails
departments. rubocop/rubocop-rspec#1848 - Introducerubocop-rspec_rails
as the cops it provides were extracted fromrubocop-rspec
and our config inrubocop_todo.yml
referenced them and rubocop doesn't like config for cops it doesn't know about so that blocked the build.