Upgrade/rails 7.1 v2 #4736
Upgrade/rails 7.1 v2 #4736
Conversation
ChargrilledChook
added
the
create-review-app
github-actions
bot
removed
the
create-review-app
There was a problem hiding this comment.
Successfuly deployed a review app on Heroku after some tweaking
This is probably a safe staging point before finishing all the config toggles - I've listed them all out in the issue.
Most will be trivial, but a few will need to be handled with care
| # Please, add to the `ignore` list any other `lib` subdirectories that do | ||
| # not contain `.rb` files, or that should not be reloaded or eager loaded. | ||
| # Common ones are `templates`, `generators`, or `middleware`, for example. | ||
| config.autoload_lib(ignore: %w[assets tasks generators]) # TODO |
There was a problem hiding this comment.
Related task in the upgrade file. Let me know if there's anything obvious that should go in here, but we'll need to come back to it to finish the upgrade.
There was a problem hiding this comment.
Most of these are low risk, some are not.
Can be worked through incrementally without doing all in one go, I reckon
| end | ||
|
|
||
| super |
There was a problem hiding this comment.
This doesn't look right, caused by Rubocop fixes. Will investigate
Actually works as expected as far as I can tell 👌
| @@ -1,7 +1,7 @@ | |||
| namespace :heroku do | |||
| desc 'Heroku release task - runs on every code push, runs before postdeploy task' | |||
| task release: :environment do | |||
| if ActiveRecord::SchemaMigration.table_exists? | |||
| if ActiveRecord::Base.connection.schema_migration.table_exists? | |||
There was a problem hiding this comment.
This was mildly annoying to debug. Caused heroku release phase to fail.
Caused by "Remove deprecated ActiveRecord::Base config accessors"
| @@ -27,7 +27,9 @@ | |||
|
|
|||
| fill_in 'Email', with: other_admin.email | |||
| fill_in 'Password', with: other_admin.password | |||
| click_button 'Sign in' | |||
| within 'form' do | |||
There was a problem hiding this comment.
Appeasing Rubocop caused these to be ambiguous. This was the easiest fix; using test-ids is probably better but I wanted to avoid touching templates for this pass
There was a problem hiding this comment.
Strange they've added this rule when it can lead to issues like this. I think click_button and click_link communicates the intent way better and avoids these unambiguous matches. Standardrb may be in our future lol
There was a problem hiding this comment.
On closer inspection this is a bit of a stitch up 😆 they've deprecrated this rule because of this exact reason.
Unfortunately the change hasn't been released yet and the rule still screams.
I'll disable the rule, revert the changes and add a todo to remove it once the library gets updated
There was a problem hiding this comment.
Nice find! stitch up indeed 😆
If its easier to keep those changes in, we can always change them back when the new release eventually appears.
There was a problem hiding this comment.
All appears to be good on the review app for me. Nice work @ChargrilledChook!
| end | ||
|
|
||
| super |
| @@ -27,7 +27,9 @@ | |||
|
|
|||
| fill_in 'Email', with: other_admin.email | |||
| fill_in 'Password', with: other_admin.password | |||
| click_button 'Sign in' | |||
| within 'form' do | |||
There was a problem hiding this comment.
Strange they've added this rule when it can lead to issues like this. I think click_button and click_link communicates the intent way better and avoids these unambiguous matches. Standardrb may be in our future lol
This commit:
* Run bundle update
* Run `app:update` task and resolve conflicts in config & env files
* Run safe autocorrect fixes for new Rubocop offences
* Add rubocop-rspec_rails to Rubcop config
* Manually fix new unsafe Rubcop offences
* Requires fixing some specs as some elements became ambiguous due to
preferring more generic `click_on` Capybara helper instead of `click_link`
* Add generators to new `autoload_lib` configuration
* Because: CI breaks otherwise
* Access schema migration object via base connection in Heroku task
* Because: This behaviour was removed in rails 7.1.
See https://github.com/rails/rails/blob/7-1-stable/activerecord/CHANGELOG.md
* Toggle low risk framework defaults for Rails 7.1
ChargrilledChook
force-pushed
the
upgrade/rails-7.1_v2
branch
from
8d76800
to
8384a65
Compare
Partially closes #4735
Supersedes #4477 - easier to start fresh than fix merge conflicts
Will rebase / squash after review