Skip to content

Conversation

@olleolleolle
Copy link
Collaborator

@olleolleolle olleolleolle commented Sep 9, 2025

This PR upgrades the configuration to Rails 7.2 configuration.

I attempted to keep close to what Rails outputs, so that the update to the next version of Rails is not so difficult.


In order to deploy this:

  • Be on Rails 7.1 defaults in the production deployed app.
  • Get to Rails 7.1 in cache settings in deployed app.
  • Fix Warnings.
  • Enable Rails 7.2 settings.

@olleolleolle olleolleolle force-pushed the rails72 branch 2 times, most recently from b591b4c to 468399b Compare September 15, 2025 20:16
@olleolleolle olleolleolle marked this pull request as ready for review November 4, 2025 18:27
@mroderick
Copy link
Collaborator

mroderick commented Nov 4, 2025

Are you planning to squash this PR when merging, to remove the non-passing commits from the git history?

Rails 7.2 app:update
Rails 7.2: enum with positional args
Rails 7.2: enum positional args
Rails 7.2: Enable 7.2 defaults
@olleolleolle
Copy link
Collaborator Author

@mroderick Now it is 1 commit.


# Disable serving static files from `public/`, relying on NGINX/Apache to do so instead.
config.public_file_server.enabled = ENV['RAILS_SERVE_STATIC_FILES'].present?
# config.public_file_server.enabled = false
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this changing the app?

Copy link
Collaborator Author

@olleolleolle olleolleolle Nov 5, 2025

Choose a reason for hiding this comment

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

The default is true. EDIT: https://guides.rubyonrails.org/configuring.html#config-public-file-server-enabled

The ENV is set on Heroku.

The result; the same.

# See the ActiveSupport::ParameterFilter documentation for supported notations and behaviors.
Rails.application.config.filter_parameters += [
:passw, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn
:passw, :email, :secret, :token, :_key, :crypt, :salt, :certificate, :otp, :ssn
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -1,51 +1,42 @@
# Puma can serve each request in a thread from an internal thread pool.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is all of this rails defaults now?

environment ENV.fetch("RAILS_ENV") { "development" }

# Specifies the `pidfile` that Puma will use.
pidfile ENV.fetch("PIDFILE") { "tmp/pids/server.pid" }
Copy link
Contributor

Choose a reason for hiding this comment

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

I see, this moved to below.


production:
adapter: redis
url: <%= ENV.fetch("REDIS_URL") { "redis://localhost:6379/1" } %>
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't have Heroku Cache currently.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right perhaps we also don't use Action Cable (websocket).

Copy link
Contributor

Choose a reason for hiding this comment

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

Why have the config then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mostly because it is automatically generated each time the upgrade dance is performed. Benefit: By having it, unused, there's less diff to manage when upgrading.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO it should at least contain a comment that it's currently not used.

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.

3 participants