Skip to content

Upgrade gemfile for Rails 5.0.7.2#298

Merged
Holmes98 merged 11 commits intomasterfrom
rails-5
Jul 5, 2025
Merged

Upgrade gemfile for Rails 5.0.7.2#298
Holmes98 merged 11 commits intomasterfrom
rails-5

Conversation

@bagedevimo
Copy link
Contributor

Upgrade gemfile for Rails 5.0.7.2

Add activemodel-serializers-xml to Gemfile

Rails 5 removed these features from core, instead using a seperate gem.

rails/rails#21161

@bagedevimo bagedevimo force-pushed the remove-strong-presenters branch from 32dab4a to 8742471 Compare May 9, 2025 20:26
@bagedevimo bagedevimo force-pushed the remove-strong-presenters branch 2 times, most recently from 1bfa5ff to f65b8e1 Compare May 9, 2025 21:41
@bagedevimo bagedevimo force-pushed the rails-5 branch 2 times, most recently from 2522099 to 331f31b Compare May 9, 2025 23:17
@bagedevimo bagedevimo force-pushed the remove-strong-presenters branch 2 times, most recently from d9fcfe6 to 5fbea5d Compare May 14, 2025 06:53
@bagedevimo bagedevimo force-pushed the remove-strong-presenters branch from 5fbea5d to 00ae31b Compare May 21, 2025 12:14
@bagedevimo bagedevimo marked this pull request as ready for review May 21, 2025 12:15
@bagedevimo bagedevimo changed the base branch from remove-strong-presenters to graphite-base/298 May 21, 2025 12:49
@bagedevimo bagedevimo force-pushed the graphite-base/298 branch from 00ae31b to 63bd463 Compare May 22, 2025 07:12
@bagedevimo bagedevimo changed the base branch from graphite-base/298 to remove-strong-presenters May 22, 2025 07:12
@bagedevimo bagedevimo force-pushed the remove-strong-presenters branch 2 times, most recently from f4d861e to a145c03 Compare May 22, 2025 12:21
@bagedevimo bagedevimo force-pushed the remove-strong-presenters branch from a145c03 to aaca701 Compare May 22, 2025 13:30
@bagedevimo
Copy link
Contributor Author

Our config/environments/production.rb doesn't currently set the log level. The default log level changes from :info to :debug in Rails 5, then back to :info in Rails 6. (References: https://guides.rubyonrails.org/v5.0.0/upgrading_ruby_on_rails.html#production-log-level, https://www.github.com/rails/rails/pull/39707)

How about we explicitly set it to :info, so that we retain the <= 4 and >= 6 behaviour?

I added a commit to that effect in branch "rails-5-suggestions" (also rebased and reworded commit message for app:update). Other than that I approve, but would appreciate another approver because I only tested lightly.

(As discussed in Discord, we will sync config/environments/*.rb separately.)

I hope you don't mind, but I re-worded that app:update commit message even more to try and explain some of my thinking at the time as well as the context you added. I also cherry-picked your :debug -> :info commit onto the top, because the ordering doesn't really matter unless we're planning on doing some kind of weird merge.

Copy link
Contributor

@tom93 tom93 left a comment

Choose a reason for hiding this comment

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

I hope you don't mind, but I re-worded that app:update commit message even more to try and explain some of my thinking at the time as well as the context you added. I also cherry-picked your :debug -> :info commit onto the top, because the ordering doesn't really matter unless we're planning on doing some kind of weird merge.

Sure. Some more nits for the commit message:

  • subject => "Run `rails app:update` and apply changes as required" (fixes typo in the command, for searching/newbies; aside: I had "bin/rails" before but we actually can't do that yet)
  • "with out modifications" => "with our modifications"(?)

I've fixed some deprecation warnings in branch "rails-5-suggestions-v2", up to you if you want to do it in this PR or a different one.

There are still a couple of warnings, including two that we'll need to fix before we upgrade to 5.1:

  1. "#table_exists? currently checks both tables and views. This behavior is deprecated and will be changed with Rails 5.1 to only check tables. Use #data_source_exists? instead."
  2. "before_filter is deprecated and will be removed in Rails 5.1. Use before_action instead."

@bagedevimo
Copy link
Contributor Author

applied your suggestions to the commit message, thanks!

yeah keen AF to merge this then start the fast-follow fixing deprecations and warnings - I have a grep ready for scanning the log files.

bagedevimo and others added 11 commits July 4, 2025 21:42
Rails 5 removed these features from core, instead using a seperate gem.

rails/rails#21161
assert_template has been removed (extracted?) to a new gem,
rails-controller-test. We use this all over the place, so this should
just work.

> Failure/Error: expect(response).to render_template("pages/forum")
> NoMethodError:
> assert_template has been extracted to a gem. To continue using it,
>		add `gem 'rails-controller-testing'` to your Gemfile.
Simple form added support for Rails 5 in 3.2.1, but only actually fixed
bugs introduced by Rails 5 in 3.3.1:

https://github.com/heartcombo/simple_form/blob/main/CHANGELOG.md#331
This presumably used to work in the prior versions of Rails or
rails-controller-test, but no longer does.
The environment configuration in config/environments/*.rb hasn't been
updated to the rails defaults with our modifications, as there are a lot
of individual changes and this is best left for future discussions and
changes.

Especially in the context of another ~8 rails versions to
upgrade to get anywhere near the latest, there will be a lot of churn in
those files and minimising that where possible is the goal here.
Rails 5 changes the ruby schema format to exclude the indexes from the
table creates, and a few other minor changes. This final is completely
generated, so it's not really something we need to review, which is why
I put it in a separate commit.
The default will change to :debug in Rails 5, and then back to :info
in Rails 6.1. Set it to :info explicitly to get stable behaviour.

(The comment mentioning personally-identifiable information is copied
from the Rails 8 template.)

References:

https://guides.rubyonrails.org/v5.0.0/upgrading_ruby_on_rails.html#production-log-level
https://www.github.com/rails/rails/pull/39707
This file is deleted later in the Rails upgrade series, so it feels a
bit silly to add it now. Additionally, having it be present with a
different value to the values we set for test and development elsewhere
causes annoying and silly issues when switching branches while we
develop and review this branch.
@Holmes98 Holmes98 merged commit 9ee5bcc into master Jul 5, 2025
9 checks passed
@Holmes98 Holmes98 deleted the rails-5 branch July 5, 2025 00:24
@puqeko puqeko mentioned this pull request Jul 8, 2025
Holmes98 added a commit that referenced this pull request Jul 11, 2025
We aren't currently using it, and the recent update in #298 seems
to have introduced issues with Qless spawning extra worker processes.
@Holmes98 Holmes98 mentioned this pull request Jul 11, 2025
Holmes98 added a commit that referenced this pull request Jul 11, 2025
We aren't currently using it, and the recent update in #298 seems
to have introduced issues with Qless spawning extra worker processes.
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.

4 participants