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

Upgrade to rails 6 #1811

Merged
merged 9 commits into from
Aug 14, 2020
Merged

Upgrade to rails 6 #1811

merged 9 commits into from
Aug 14, 2020

Conversation

cdccollins
Copy link
Contributor

@cdccollins cdccollins commented Jul 3, 2020

Upgrade to Rails 6, with minor config changes that come with it. Have also removed specified gem versions in Gemfile as it doesn't seem to cause any issues.

⚠️ This application is Continuously Deployed: ⚠️

  • Merged changes are automatically deployed to staging and production.

  • Make sure you follow the guidance for deployments before you merge.

  • Check your branch is being deployed in the Release app, after merging.

@bevanloon bevanloon temporarily deployed to government-f-upgrade-to-eiuc43 July 3, 2020 14:14 Inactive
@cdccollins cdccollins marked this pull request as draft July 3, 2020 14:23
@cdccollins cdccollins force-pushed the upgrade-to-rails-6 branch from b4f1a0b to 4c008f1 Compare July 8, 2020 09:40
@bevanloon bevanloon temporarily deployed to government-f-upgrade-to-eiuc43 July 8, 2020 09:40 Inactive
config/application.rb Outdated Show resolved Hide resolved
config/environments/development.rb Outdated Show resolved Hide resolved
config/environments/production.rb Outdated Show resolved Hide resolved
@cdccollins cdccollins force-pushed the upgrade-to-rails-6 branch from 4c008f1 to bbad063 Compare July 9, 2020 13:26
@bevanloon bevanloon temporarily deployed to government-f-upgrade-to-eiuc43 July 9, 2020 13:26 Inactive
@cdccollins cdccollins force-pushed the upgrade-to-rails-6 branch from bbad063 to 5a322e1 Compare July 9, 2020 13:31
@bevanloon bevanloon temporarily deployed to government-f-upgrade-to-eiuc43 July 9, 2020 13:32 Inactive
@cdccollins cdccollins marked this pull request as ready for review July 9, 2020 13:43
@cdccollins cdccollins force-pushed the upgrade-to-rails-6 branch from 5a322e1 to 3548916 Compare July 16, 2020 09:27
Copy link

@pudiva pudiva left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just needs to resolve the conflicts and :shipit:

When running tests I was getting this error:
`ActionView::Template::Error: undefined method `bubbles?' for true:Sass::Script::Tree::Literal`

The solution is suggested here sds/scss-lint#278, specifically
"You should only require scss-lint explicitly in your code so that the monkey patching code
doesn't get loaded automatically. You can then explicitly load it for the cases where you need it"

Putting it first in the rake tasks seems to stop it erroring as well.
Pin rails to specific version and make it stand out, see
alphagov/service-manual-frontend@27390cd for reasoning.
All the changes that come with Rails 6, mainly comments and minor config changes.
Custom alias domains such as http://government-frontend.dev.gov.uk are not
working on any apps running Rails 6, due to the addition of "Host
Authorisation" middleware so we need to explicitly add the local domain
to their `config.hosts`.

Similar PR: alphagov/info-frontend#644

Issue on govuk-docker: alphagov/support#687
According to testdouble/jasmine-rails#238 the jasmine-rails gem is no
longer actively maintained and is reliant on the phantomjs headless
browser which has suspended development (ariya/phantomjs#15344).
Therefore it makes more sense to switch to the jasmine gem and use the
jasmine_selenium_runner gem to switch to headless Chrome for testing.
Using a sass css compressor causes a scss file to be processed twice
(once to build, once to compress) which breaks the usage of "unquote"
to use CSS that has same function names as SCSS such as max.
To work around the issue with sassc and unquote the accepted solution has been to
set css_compressor to nil and then use sass_style to achieve a similar effect.
However the gem forces the css_compressor back to :sass thus breaking again on asset compilation.

Removing the gem fixes this and the gem itself does not seem to be necessary since
it was introduced to fix issues in Firefox 52 and the latest Firefox version is
78
@benthorner
Copy link
Contributor

⚠️ This application is Continuously Deployed: ⚠️

  • Merged changes are automatically deployed to staging and production.

  • Make sure you follow the guidance for deployments before you merge.

  • Check your branch is being deployed in the Release app, after merging.

@thomasleese thomasleese merged commit 8a4be3b into master Aug 14, 2020
@thomasleese thomasleese deleted the upgrade-to-rails-6 branch August 14, 2020 09:17
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.

5 participants