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

[#5159] Rails 6 upgrade #6485

Merged
merged 3 commits into from
Sep 29, 2021
Merged

[#5159] Rails 6 upgrade #6485

merged 3 commits into from
Sep 29, 2021

Conversation

gbp
Copy link
Member

@gbp gbp commented Aug 18, 2021

Relevant issue(s)

Fixes #5159

What does this do?

  1. Upgrade Alaveteli to Rails 6.0
  2. Refactor gettext/locale loading to resolve app booting issue in development environment.

Notes to reviewer

This PR doesn't prep for the Rails 6.1 release as the CI will be red and with how Github actions works we can't allow failures without it putting red crosses everywhere and emailing us. We don't really want that so lets prep in a separate PR and fix CI there.

@gbp gbp changed the title WIP Rails 6 upgrade [#5159] Rails 6 upgrade Sep 7, 2021
lib/alaveteli_localization.rb Outdated Show resolved Hide resolved
@gbp gbp marked this pull request as ready for review September 7, 2021 13:49
@gbp gbp requested a review from garethrees September 7, 2021 13:49
Copy link
Member

@garethrees garethrees left a comment

Choose a reason for hiding this comment

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

0ca6063 – Nice!

What are we doing about the new autoloading stuff? Is that getting handled in #5851 or do they need to be addressed here?

Not much change so fine in principle. I've booted the app successfully but haven't tested further than that.

lib/alaveteli_localization.rb Outdated Show resolved Hide resolved
lib/alaveteli_localization.rb Outdated Show resolved Hide resolved
lib/alaveteli_localization.rb Outdated Show resolved Hide resolved
Rails 5.2 is no more so we can drop the old implementation.

Leave conditionals in Gemfiles for when we prep for Rails 6.1.
In development mode Rails might output some deprecation warnings which
don't appear in test or production. It seems Rails 6 prepares these
warnings ahead of time and they rely on the `#to_sentence` this in turn
loads the locales and but because our gettext based strings are loaded
in an initializer the available locales aren't yet set. Resulting in a
error being raised with our i18n fixes.

This change moves all the gettext and locale setup into a Railtie and
sets everything up in a `before_configuration` block which runs before
any initializers are run.

Fixes:
```
./lib/i18n_fixes.rb:54:in `available_locales': undefined method `map' for nil:NilClass (NoMethodError)
```
@gbp
Copy link
Member Author

gbp commented Sep 29, 2021

What are we doing about the new autoloading stuff? Is that getting handled in #5851 or do they need to be addressed here?

This isn't using Zeitwerk yet. We would need load defaults for Rails "6.0" in config/application.rb to enable

@gbp gbp merged commit 970f75f into develop Sep 29, 2021
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.

Upgrade to Rails 6.0
2 participants