Skip to content

Upgrade: Rails 7.2 → 8.1 & Ruby 3.2.9 → 4.0.1#196

Open
kaysiz wants to merge 10 commits into
masterfrom
ks-ruby-rails-8-upgrade
Open

Upgrade: Rails 7.2 → 8.1 & Ruby 3.2.9 → 4.0.1#196
kaysiz wants to merge 10 commits into
masterfrom
ks-ruby-rails-8-upgrade

Conversation

@kaysiz
Copy link
Copy Markdown
Member

@kaysiz kaysiz commented May 11, 2026

Purpose

closes: Add github issue that originated this PR

Approach

Open Questions and Pre-Merge TODOs

Learning

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

@kaysiz kaysiz self-assigned this May 11, 2026
Copy link
Copy Markdown
Contributor

@richardhallett richardhallett left a comment

Choose a reason for hiding this comment

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

Overall looks good, I think i'm a little unsure of the request_encoding change, but might be fine.

end

if exists && params[:compressed].present?
if exists && request_encoding == "gzip"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this will work, but is the request_encoding method always returning a string "gzip" for the various options?

I'm a little unsure about changing it from the check on the param :compressed, though that might end up being the same?

# config.action_mailer.perform_caching = false
# Log to STDOUT with the current request id as a default log tag.
config.log_tags = [ :request_id ]
config.logger = ActiveSupport::TaggedLogging.logger(STDOUT)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will override the lograge setup in application.rb

# Set this to true and configure the email server for immediate delivery to raise delivery errors.
# config.action_mailer.raise_delivery_errors = false
# Change to "debug" to log everything (including potentially personally-identifiable information!).
config.log_level = ENV.fetch("RAILS_LOG_LEVEL", "info")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See above, set at the application.rb file, env var is different.

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