Skip to content

Rails 8 upgrade#991

Merged
santib merged 16 commits intomainfrom
rails-8-upgrade
Mar 4, 2025
Merged

Rails 8 upgrade#991
santib merged 16 commits intomainfrom
rails-8-upgrade

Conversation

@santir489
Copy link
Contributor

@santir489 santir489 commented Feb 26, 2025

Description:

Rails 8 upgrade.

  • devise_auth_token does not have a release with Rails 8 support yet.
  • annotate gem is not compatible with rails 8, and is not maintained. Replaced it with annotaterb

Notes:


Tasks:

  • Add each element in this format

Risk:


Preview:

'exclude_factories' => 'false',
'ignore_model_sub_dir' => 'false',
'skip_on_db_migrate' => 'false',
'format_bare' => 'true',
Copy link
Contributor Author

@santir489 santir489 Feb 26, 2025

Choose a reason for hiding this comment

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

format_bare was the only config missing in .annotaterb.yml, so I added manually

@@ -0,0 +1,59 @@
---
Copy link
Contributor Author

Choose a reason for hiding this comment

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

autogenerated file

Copy link
Member

Choose a reason for hiding this comment

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

autogenerated but adjusted given the configs in lib/tasks/auto_annotate_models.rake right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct!

@santir489 santir489 marked this pull request as ready for review February 26, 2025 14:40
Copy link
Contributor

@JulianPasquale JulianPasquale left a comment

Choose a reason for hiding this comment

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

👏 👏

@JulianPasquale JulianPasquale requested a review from a team February 26, 2025 14:48
Copy link
Member

@santib santib left a comment

Choose a reason for hiding this comment

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

Did you follow the steps in https://guides.rubyonrails.org/upgrading_ruby_on_rails.html ?

I see some steps missing, like upgrading the Rails JS packages, and running the app "update task"

Ideally, any default that we have (and did not modify on purpose) should be updated with any new Rails default (e.g. this happens a lot in config/environments/* files, initializers, etc). So we need to do like a merge of these files as explained in the link above


puts "\n== Everything is good to go! =="

unless ARGV.include?('--skip-server')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure if this is necessary 🤔
@santib thoughts ?

Copy link
Member

Choose a reason for hiding this comment

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

if this is Rails default then I'd leave it. I guess they extended the setup script so that it also starts the server by default which sounds reasonable to me

Copy link
Contributor

@jpascual1994 jpascual1994 left a comment

Choose a reason for hiding this comment

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

🙌

bin/setup Outdated
Comment on lines 33 to 34
# system "ln -nfs #{APP_ROOT} ~/.puma-dev/#{APP_NAME}"
# system "curl -Is https://#{APP_NAME}.test/up | head -n 1"
Copy link
Member

@santib santib Feb 27, 2025

Choose a reason for hiding this comment

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

these two lines should be removed as well

Copy link
Member

@santib santib left a comment

Choose a reason for hiding this comment

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

👏 thank you!

@santib santib merged commit 7474d9e into main Mar 4, 2025
3 of 4 checks passed
@santib santib deleted the rails-8-upgrade branch March 4, 2025 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants