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

Rails 8 #2319

Merged
merged 6 commits into from
Feb 5, 2025
Merged

Rails 8 #2319

merged 6 commits into from
Feb 5, 2025

Conversation

KludgeKML
Copy link
Contributor

@KludgeKML KludgeKML commented Feb 3, 2025

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

https://trello.com/c/9ymFymOL/249-owned-app-rails-8-updates

- Tests complain here that the options array being passed
  in is frozen. Replace mutation with a merge and return
  pattern to avoid this issue.
- Match criteria seem to have stabilised order (this
  now matches the order we ask for them when converting
  the matching criterai to json here:
  https://github.com/alphagov/email-alert-api/blob/5b4be94377f3dc78baf1df15bd244bd3946fa960/lib/reports/subscriber_lists_report_row.rb#L28-L35
  ...so it seems reasonable to accept this update.
@KludgeKML KludgeKML requested a review from deborahchua February 5, 2025 10:51
@KludgeKML KludgeKML marked this pull request as ready for review February 5, 2025 10:51
Copy link
Contributor

@deborahchua deborahchua left a comment

Choose a reason for hiding this comment

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

LGTM

@KludgeKML KludgeKML merged commit 98ac792 into main Feb 5, 2025
9 checks passed
@KludgeKML KludgeKML deleted the rails-8 branch February 5, 2025 12:23
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.

2 participants