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

Allow bundling with local govuk_frontend_toolkit #1035

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion Gemfile
Expand Up @@ -41,7 +41,12 @@ group :development, :test do
end

gem 'plek', '1.11.0'
gem 'govuk_frontend_toolkit', '~> 6.0.3'

if ENV['GOVUK_FRONTEND_TOOLKIT_DEV']
gem 'govuk_frontend_toolkit', path: "./tmp/govuk_frontend_toolkit_gem_dev"
else
gem 'govuk_frontend_toolkit', '~> 6.0.3'
end

if ENV['GOVUK_TEMPLATE_DEV']
gem 'govuk_template', path: "../govuk_template"
Expand Down
3 changes: 3 additions & 0 deletions README.md
Expand Up @@ -53,6 +53,9 @@ are wrapped around feedback pages, run `bowl feedback
in a separate terminal. Following local edits to `static`, restarting just
`feedback` should be sufficient.

#### Testing a local version of govuk_frontend_toolkit
To test any local updates to [govuk_frontend_toolkit](https://github.com/alphagov/govuk_frontend_toolkit) you can use `./startup.sh --test-govuk-frontend-toolkit`

### Running the test suite

`bundle exec rake` runs the test suite.
Expand Down
28 changes: 27 additions & 1 deletion startup.sh
@@ -1,4 +1,30 @@
#!/bin/bash
#!/bin/bash

bundle install

if [[ $1 == "--test-govuk-frontend-toolkit" ]] ; then
# Find out where it the gem is installed
installed_location="$(bundle show govuk_frontend_toolkit)"
temporary_location="./tmp/govuk_frontend_toolkit_gem_dev"

# Remove any existing tmp file
rm -rf ${temporary_location}
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels mildly dangerous


# Copy the old assets aside
# Using sudo here since the installed location has elevated permissions
sudo cp -r ${installed_location} ${temporary_location}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is sudo needed here?

Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be good for the comment to include why they are being copied

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mainly because I didn't want to sudo rm -rf I think

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you update the comment so it’s clear?


# Remove current submoduled assets
rm -rf ${temporary_location}/app/assets

# Symlink the local
sudo ln -rs ../govuk_frontend_toolkit ${temporary_location}/app/assets
Copy link
Contributor

Choose a reason for hiding this comment

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

In my local govuk_frontend_toolkit checkout I've been running the tests so I have a node_modules folder which is copied over as part of this command. This folder is huge, and contains coffee script assets. This has two repercussions:

  1. the requests to static take a very long time as they cache all the files in node_modules
  2. the requests to static fail because we don't include the coffee script gem, but there are coffee script files in the node_modules folder which is part of the assets path now and sprockets breaks when trying to load a coffee script processor for them.

If we add a rm -rf ${temporary_location}/app/assets/node_modules this gets round the problem, but it means we've deleted them from the local checkout to which is pretty surprising. If we change ln -rs to cp -r as well that works, but we'd have to keep remembering to run this script whenever we make a local change to govuk_frontend_toolkit.

Not sure what the best solution is TBH.


export GOVUK_FRONTEND_TOOLKIT_DEV=true

bundle install

echo "Testing local govuk_frontend_toolkit"
fi

bundle exec rails s -p 3013