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

Rails 5 upgrade #5982

Merged
merged 27 commits into from Feb 16, 2016
Merged

Rails 5 upgrade #5982

merged 27 commits into from Feb 16, 2016

Conversation

matthewd
Copy link
Contributor

No description provided.

@miq-bot
Copy link
Member

miq-bot commented Dec 23, 2015

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@@ -1,7 +1,7 @@
class CimBaseStorageExtent < ActsAsArModel
set_columns_hash(CimStorageExtent.columns_hash.keys.each_with_object({}) { |c, h| h[c.to_sym] = CimStorageExtent.columns_hash[c].type })

def self._virtual_colums_hash
def self._virtual_columns_hash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this code isn't being used 😉

@Fryguy
Copy link
Member

Fryguy commented Dec 23, 2015

Awesome work @matthewd ! Can any of this be extracted and used ahead of time ni the Rails 4 code base? I'm looking at things like the helper changes in the helper specs as an example.

cc @tenderlove

@matthewd matthewd force-pushed the rails5 branch 2 times, most recently from 69e2a19 to 8e4f313 Compare December 23, 2015 20:32
@matthewd
Copy link
Contributor Author

Can any of this be extracted and used ahead of time in the Rails 4 code base?

Yeah.. I'll plan to extract a couple (like the helpers) which seem conflict-prone to keep separate. For most, even if they're 4.2-compatible, it shouldn't matter once I tame VirtualColumn and get this green.

@@ -4,8 +4,9 @@ eval_gemfile(File.expand_path("gems/pending/Gemfile", __dir__))
# VMDB specific gems
#

gem "activerecord-deprecated_finders", "~>1.0.4", :require => "active_record/deprecated_finders"
gem "rails", "~>4.2.5"
gem "rails", :git => "git://github.com/matthewd/rails.git", :branch => "actioncontroller-hax"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😨 Seems like a legit branch name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -4,8 +4,9 @@ eval_gemfile(File.expand_path("gems/pending/Gemfile", __dir__))
# VMDB specific gems
#

gem "activerecord-deprecated_finders", "~>1.0.4", :require => "active_record/deprecated_finders"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please tell me this is gone for real. 😍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I want deprecated_finders gone yet.
Is there a way we can bring these in for automate?

Or do we have a different plan?

end
scope.where(cond_for_count).includes(includes).references(includes).count
if conditions && extra_target_ids
scope = scope.where(conditions).or(scope.where(:id => extra_target_ids))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

Matthew Draper added 2 commits February 16, 2016 00:20
I'd love to kill it off and switch to nice relations everwhere, but
we're just not close enough to pull that off at the moment: e.g., these
values get stored in the session.
@matthewd matthewd force-pushed the rails5 branch 2 times, most recently from ad98650 to 2623547 Compare February 15, 2016 14:10
@miq-bot
Copy link
Member

miq-bot commented Feb 15, 2016

<github_pr_commenter_batch />Some comments on commits matthewd/manageiq@005f257~...9629756

@miq-bot
Copy link
Member

miq-bot commented Feb 15, 2016

Checked commits matthewd/manageiq@005f257~...9629756 with ruby 2.2.3, rubocop 0.34.2, and haml-lint 0.13.0
61 files checked, 64 offenses detected

Gemfile

app/controllers/application_controller.rb

app/controllers/container_controller.rb

app/controllers/mixins/vm_show_mixin.rb

app/controllers/provider_foreman_controller.rb

app/controllers/service_controller.rb

app/models/ems_refresh/save_inventory_helper.rb

app/models/miq_expression.rb

app/models/miq_report/generator.rb

app/models/mixins/reserved_mixin.rb

app/models/vim_usage.rb

app/presenters/explorer_presenter.rb

config/environments/test.rb

lib/acts_as_ar_model.rb

lib/extensions/ar_adapter/ar_dba/postgresql.rb

lib/extensions/ar_virtual.rb

spec/controllers/catalog_controller_spec.rb

spec/controllers/miq_policy_controller_spec.rb

spec/lib/extensions/ar_to_model_hash_spec.rb

spec/lib/extensions/ar_virtual_spec.rb

spec/models/vmdb_database_spec.rb

klass.predicate_builder.build_from_hash(cond).map { |b|
klass.connection.visitor.compile b
}.join(' AND ')
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a nice compromise. this will help for now.

Fryguy added a commit that referenced this pull request Feb 16, 2016
@Fryguy Fryguy merged commit 9b0e373 into ManageIQ:master Feb 16, 2016
@Fryguy Fryguy added the core label Feb 16, 2016
@Fryguy Fryguy added this to the Sprint 37 Ending Mar 8, 2016 milestone Feb 16, 2016
@chrisarcand
Copy link
Member

🎉 🎉 🎉

👏 @matthewd

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

Successfully merging this pull request may close these issues.

None yet

7 participants