Skip to content

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

Open
kaysiz wants to merge 29 commits intomasterfrom
ks-rails-8-upgrade
Open

Upgrade: Rails 7.2 → 8.1 & Ruby 3.2.9 → 4.0.1#1506
kaysiz wants to merge 29 commits intomasterfrom
ks-rails-8-upgrade

Conversation

@kaysiz
Copy link
Copy Markdown
Member

@kaysiz kaysiz commented Apr 7, 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 Apr 7, 2026
@kaysiz kaysiz added the run-all-tests Run All CI Tests label Apr 7, 2026
@kaysiz kaysiz changed the title Ks rails 8 upgrade Upgrade: Rails 7.2 → 8.1 & Ruby 3.2.9 → 4.0.1 Apr 13, 2026
@kaysiz kaysiz requested a review from a team April 13, 2026 12:03

# Logger configuration - uses LogStashLogger (structured JSON logging)
# Rails.logger and Rails.application.config.lograge.logger are the same instance
config.logger = Rails.logger
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.

What's the removal of this for? I think this was in place as per the comment to use same setup for Rails with sentry, did something change in a gem?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The gem does not have logger attribute anymore.

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.

Hmm okay, then I guess we need to check it integrates okay on staging.

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.

Lots of files changed but mostly seem similiar so I think fine.
Once merged we'll need to do some testing but PR itself looks okay to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-all-tests Run All CI Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants