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.0/5.1 prepare_binds_for_database is gone #18078

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Oct 10, 2018

To avoid relying on the connection to type cast the binds, this method was
removed. It was replaced with the underlying implementation of the
method, which is also used here to replace our usage.

rails/rails@5465508

Extracted from #18076

To avoid relying on the connection to type cast the binds, this method was
removed.  It was replaced with the underlying implementation of the
method, which is also used here to replace our usage.

rails/rails@5465508

Extracted from ManageIQ#18076
@jrafanie jrafanie force-pushed the rails_5_1_prepare_binds_for_database_was_removed branch from 9bf9dc2 to 651d6a0 Compare October 10, 2018 18:27
@jrafanie jrafanie requested a review from kbrock October 10, 2018 18:33
Copy link
Member

@NickLaMuro NickLaMuro left a comment

Choose a reason for hiding this comment

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

This looks good to me. The change matches what is being done in the Rails commit, so seems good to me.

On a side note: I noticed they were doing an unnecessary "double map" in a few places now as a result of the change (in Rails core that is), so I might commit a patch for that (assuming it is still there)!

😉

Anyway, nice work! 👍

@miq-bot
Copy link
Member

miq-bot commented Oct 12, 2018

Checked commit jrafanie@651d6a0 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 👍

@kbrock kbrock self-assigned this Oct 12, 2018
@kbrock kbrock added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 12, 2018
@kbrock kbrock merged commit 019dd8d into ManageIQ:master Oct 12, 2018
@jrafanie jrafanie deleted the rails_5_1_prepare_binds_for_database_was_removed branch December 14, 2018 17:35
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.

4 participants