-
Notifications
You must be signed in to change notification settings - Fork 16
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
Bump rails from 7.2.2.1 to 8.0.1 #3071
Conversation
This application is owned by the publishing platform team. Please let us know in #govuk-publishing-platform when you raise any PRs. Follow these steps if you are doing a Rails upgrade. |
Covered by this Trello card. |
321a8f3
to
ca4ff07
Compare
6e94fd3
to
cb30e3a
Compare
Bumps [rails](https://github.com/rails/rails) from 7.2.2.1 to 8.0.1. - [Release notes](https://github.com/rails/rails/releases) - [Commits](rails/rails@v7.2.2.1...v8.0.1) --- updated-dependencies: - dependency-name: rails dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
The old syntax had already been deprecated, but it was fully removed in Rails 8: https://edgeguides.rubyonrails.org/8_0_release_notes.html#active-record-removals Note that the way we're defining a value (as a string) might have a performance impact: https://api.rubyonrails.org/v8.0.1/classes/ActiveRecord/Enum.html#:~:text=Finally%20it%E2%80%99s%20also%20possible%20to%20use%20a%20string%20column%20to%20persist%20the%20enumerated%20value.%20Note%20that%20this%20will%20likely%20lead%20to%20slower%20database%20queries%3A
This appears to have changed in Rails 8: rails/rails@e1d58cf (see `actionpack/lib/action_controller/metal/strong_parameters.rb`)
I ignored most changes to config/application.rb and config/puma.rb, except ignoring assets and tasks subdirectories when autoloading the lib directory. This was ignored in [the 7.1 upgrade](0b02a4f), but it looks like all other alphagov apps with this config include these default ignored subdirectories I retained the deprecation warnings in dev and test, since it seems sensible to be warned about deprecated code. Upstream PR that removed this: rails/rails#51831 In production, I decided to retain the evented file watcher. I added this to this app in 60715d0, after the 7.2.0 upgrade, but I'm not clear why. It was removed from the default Rails config in rails/rails@e34300a, released in 7.0.0. However, a comment on that PR noted that the evented file watched does sometimes perform better than the implicit default: rails/rails#42985 (comment). I've run the the benchmarks reported in that comment (loading the app's models) on my M2 Air and found a smaller difference but still consistently in favour of the evented file watcher. In three tests, the non-evented file watcher took 1.88, 4.20, and 2.42 milliseconds, whereas the evented one took 0.16, 0.02, and 0.02 milliseconds Also in production, I commented out the new setting `config.silence_healthcheck_path = "/up"`. We use '/healthcheck' rather than `/up`, and I'm not sure that we'd want to silence logs from this I skipped the attributes_for_inspect change, per the [Signon upgrade](alphagov/signon@fb0b346) A few settings that we never changed from their defaults were changed in 7.1.0, but not in our app. Our git history doesn't explain why they were ignored, so I'm updating them now: - logging settings - public file server settings - `config.force_ssl = true`
cb30e3a
to
2229c51
Compare
Tested creating and force publishing a new draft in integration via Whitehall with this branch deployed and all worked fine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes all look good to me.
Trello
Closes #3080
Bumps rails from 7.2.2.1 to 8.0.1.
Release notes
Sourced from rails's releases.
... (truncated)
Commits
cf6ff17
Preparing for 8.0.1 release0bba3c2
Merge pull request #53936 from jsharpify/jsharpify/prism-parsing8521b99
[RF-DOCS] Update Rails Testing Guide [ci skip] (#53872)dbe61a7
Merge pull request #53907 from p8/guides/fix-canonicalf8d559d
[RF-DOCS] Asset Pipeline Documentation (Propshaft) [ci-skip] (#53875)2ae1d69
[RF-DOCS] Solid Cache updates in Caching with Rails: An Overview [ci-skip] (...85bde83
Merge pull request #53926 from Ridhwana/Ridhwana/solid-queuec167cbe
Merge pull request #53941 from byroot/rack-server-protocol656e209
Merge pull request #53940 from mjankowski/rdoc-link-to-add-check-constraintb9608a6
Merge pull request #53937 from fatkodima/mysql-fix-remove-foreign-key-restrictDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions
will show all of the ignore conditions of the specified dependency@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)