Rails 6.0 #95
Rails 6.0 #95
Conversation
| @@ -22,7 +22,7 @@ Gem::Specification.new do |spec| | |||
| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } | |||
| spec.require_paths = ["lib"] | |||
|
|
|||
| spec.add_dependency "activerecord", "~> 5.0" | |||
There was a problem hiding this comment.
Can we apply https://github.com/ManageIQ/activerecord-id_regions/pull/18/files#r537735807 here as well?
There was a problem hiding this comment.
Yup, agreed. Since this is a gem we publish, and there is seemingly no other change, just supporting Rails 5 and 6 should be sufficient for our purposes.
There was a problem hiding this comment.
Not sure we can do this now, since there is a .update_attributes change I did. I probably can remove that change, but wanted to get everyone else's opinion before I do.
There was a problem hiding this comment.
@NickLaMuro it looks like the only changes from #update_attributes to #update were in specs, I'm good with ">=5.0", "< 6.1"
There was a problem hiding this comment.
Good point. I think I saw lib/inventory_refresh/inventory_collection/helpers/initialize_helper.rb in the files changed diff, and didn't take a look at the contents, which only changes a comment and is fine.
Allow the 2.x release to support Rails 6.0 applications as well.
|
cc @slemrmartin @NickLaMuro once this is passing specs I'm good with this if Martin is good from the topo side. As for MIQ we'll need to backport it to the v2 branch |
|
@agrare I will look into the failures. That said...
This PR is already targeting the |
|
Gah sorry I missed that this was to the v2 branch I just assumed it was master my mistake |
|
No worries, I did the same when I originally opened up the fix months ago, so it wasn't a wrong assumption to make, just one that I already have addressed (since it was causing major failures in cross-repo if I hadn't already solved for this). |
|
Hi @agrare yeah this PR won't affect us as it won't be merged to 0.3.z. We'll upgrade Rails probably in the Spring |
Due to a performance fix introduced in Rails 6: rails/rails@cc2d614e Columns named "present" end up with the following error: ActiveRecord::DangerousAttributeError: present? is defined by Active Record. Check to make sure that you don't have an attribute or method with the same name. In manageiq, we kinda punted on fixing this column since it is hard to know where it might be used elsewhere (Automate, etc.) so if a rename is the final route we code with it is something that ideally happens in a targeted effort outside the Rails 6 upgrade. But here, since it is only example models for specs, changing here isn't as dangerous so being cavalier about the change isn't really an issue.
Seems like in `Rails.6`, `String#strip_heredoc` isn't available unless `active_support/all` is explicitly required. Requiring in the `spec_helper.rb` is the most pragmatic solution and will apply it for all specs.
When running the specs, the following type of error was presented:
DEPRECATION WARNING: Static attributes will be removed in FactoryBot
5.0. Please use dynamic attributes instead by wrapping the attribute
value in a block:
cpu_sockets { 1 }
To automatically update from static attributes to dynamic ones,
install rubocop-rspec and run:
rubocop \
--require rubocop-rspec \
--only FactoryBot/AttributeDefinedStatically \
--auto-correct
(called from block (3 levels) in <top (required)> at ./inventory_refresh/spec/factories/hardware.rb:4)
The result of this commit is what was generated via:
bundle exec rubocop --require rubocop-rspec \
--only RSpec/FactoryBot/AttributeDefinedStatically \
--auto-correct
Fixing the following deprecation warning: DEPRECATION WARNING: update_attributes is deprecated and will be removed from Rails 6.1 (please, use update instead)
|
Checked commits NickLaMuro/inventory_refresh@628dfe8~...5ce534d with ruby 2.6.3, rubocop 0.82.0, haml-lint 0.35.0, and yamllint **
|
|
Released v0.2.3 with this change |
|
I should mention I'm very excited about the ability to use this with rails 6 because of the introduction of |
|
@agrare Nice! The cross-repo PR for Rails 6.0 is now running with all of these newly released gems: ManageIQ/manageiq-cross_repo-tests#179 I think those were the last hold out for merging them into master... 🤞 |
Rails 6.0 (cherry picked from commit e88e6bc)
Based off the
v0.2.zbranch, since master isv0.3.z+Links