-
Notifications
You must be signed in to change notification settings - Fork 6
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
Upgrade to Rails 6 #1709
Upgrade to Rails 6 #1709
Conversation
1342b28
to
43ee2b3
Compare
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.
Looks great, apart from the Gemfile merge conflicts that have arrived 🙈
00b08c3
to
1631a68
Compare
1631a68
to
5a32942
Compare
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.
We've got a linting fail now because listen
in the Gemfile is not in alphabetical order 😣
6aa0a2e
to
77b456f
Compare
Another Gemfile conflict! 😱 |
Using a beta version of rspec-rails due to an upstream bug: we should switch to 4.0.0 when that's out.
This wasn't covered by the app:update task because this is a custom initialiser.
This fixes a potential RCE exception, though the app doesn't set cookies.
We can re-enable this when libsass adds support for min/max.
The app fails to start in govuk-docker without this: /[etc]/activesupport-6.0.0/lib/active_support/dependencies.rb:325: in `require': Could not load the 'listen' gem. Add `gem 'listen'` to the development group of your Gemfile (LoadError)
77b456f
to
b5d3042
Compare
🚀 |
We should gradually switch to the new defaults once we're happy with this.
There's one deprecation warning which bothers me:
But... according to
git grep parent_name
, it's not referenced anywhere. Must be in a dependency, in which case hopefully it'll be updated before then.Trello card
Search page examples to sanity check:
Other finders