Rails71 #1426
Rails71 #1426
Conversation
bundle update rails
- inflections.rb was not fully accepted. It leaves one inflection we added. This should have been done separately and not been included in this commit.
Accepted additions, kept our modifications (flipflop and delayedjob)
- accepted additions and syntax changes - kept our flipflip config - letter opener config - log settings
- kept our flipflop - kept our bullet config - kept our ENV settings - accepted additions/syntax changes
- kept our administrate precompile
updated an old migration that has caused issues over time It shouldn't affect existing databases, but should allow fresh db loads to have the correct schema
Rails new setting detects that this except doesn't exist. Rails is correct, so I removed it. https://www.shakacode.com/blog/rails-adds-ability-to-raise-error-on-missing-callback-actions/
We still need: `Rails.application.config.active_record.run_after_transaction_callbacks_in_order_defined = false` I'm not sure how long this setting will stick around as an option (i.e. when we upgrade to 8.x we may have to adjust our callbacks)
We had a test fail in CI that passes locally. This is an attempt to see if CI will pass in this mode so we could land 7.1 and then introduce the 7.1 defaults over time.
We had to do this in Bento as well: MITLibraries/bento@6b7b24c
jazairi
left a comment
jazairi
left a comment
There was a problem hiding this comment.
Thanks for picking this up, and thanks for the updates about this work in Slack and the ticket! It was helpful to come into this review with some awareness of your process.
While I'd prefer to be using 7.1 framework defaults, I agree that running 7.1 with 7.0 defaults is a significant improvement for maintenance purposes. I think the key will be remembering that we're not using 7.1 defaults, but I think that leaving the new_framework_defaults file in the repo should help with that.
That db migration made me a tad nervous, too, so I like your idea of backing up prod prior to promotion.
Otherwise, everything here is consistent with my expectations. (It will always be hilarious to me that rubocop and Rails disagree on single quotes vs double quotes.)
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Requires database migrations?
YES
Includes new or updated dependencies?
YES