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

Start tracking upstream prototype kit, and upgrade to kit v8.6 #28

Merged
merged 1,433 commits into from
Feb 4, 2019
Merged

Conversation

fofr
Copy link
Contributor

@fofr fofr commented Feb 4, 2019

Following instructions here:
https://govuk-prototype-kit.herokuapp.com/docs/updating-the-kit

Used --allow-unrelated-histories to bring the two inline.

See 84cac44 for actual changes. Puts back /docs for ease of future upgrades.

jfranciswebdesign and others added 30 commits July 18, 2018 12:38
Currently, if a user enters an answer on examples/pass-data/vehicle-registration and then clicks 'Change' on examples/pass-data/vehicle-check-answers the previously filled answer is lost. Adding in the data element would show the user's answer back to them (the radio button and checkbox example pages work as expected).
- Use post and session data instead of get and query data
- Tidy up code by putting example files in a folder
basic-auth 2.0.0 drops support for Node.js below 0.8, but we require Node 8.0.It also removes support for passing the context directly to the auth function, but that's ok because we pass the request object.

Tested by adding the following to the .env file and ensuring that basic auth still functions:

USERNAME=foo
PASSWORD=bar
NODE_ENV=production
USE_HTTPS=false
cross-spawn 6.0 removes support for older node versions, only node >= 4 is supported. This is OK because we already require node >= 8.

Tested by making sure that the Prototype Kit still runs without error.
dotenv 6.0 drops support for node v4, but this is fine because we already require node v6.

Other breaking changes are:
- default path is now path.resolve(process.cwd(), '.env')
- does not write over keys already in process.env if the key has a falsy value

Neither of these should affect the kit.

Tested by adding `FOO=bar` to the `.env` file, and ensuring that it is available within the app by adding a `console.log(process.env.FOO)` and checking for 'bar' in the console output.
Only bugfixes, features and dependency updates.
Tested by running `gulp clean` and checking that the public directory is removed.
There are no release notes or changelog for this module, but looking at sindresorhus/gulp-mocha@v4.0.0...v6.0.0 v6.0 drops support for anything below Node 6. This should be fine, as we require Node 8 or above anyway.

Once updated, the tests started 'hanging' – updating the mocha gulp task to use the example from their readme (passing exit: true) solved this.
gulp-sass 4.0 drops support for Node < 4, but this is fine because we require node 8 anyway.

Tested by running `gulp clean` to remove the public directory, then running `npm start` and ensuring that the stylesheet is re-generated and the prototype kit app looks correct.
Fixes a number of security vulnerabilities, fixes a load of bugs and adds a number of new features.

Some of these are listed as breaking changes, but only because they fix bugs so the output would be different.

Tested by going through the documentation (which is written in markdown) and checking that it looks OK.
This package isn't actually require'd or used anywhere in the application – it's pre-installed to make it easier for prototype kit users to get setup with Notify.
This is no longer used as of 6ff3b6b.
Tested by ensuring that gulp tasks are still included from the gulp directory (the only place that require-dir is used is in gulpfile.js, where it includes the tasks from ./gulp.
Tested by ensuring that the gulp tasks in gulp/tasks.js that use run-sequence still run without error.
Tested by ensuring that the release url on the `/docs/install` page is still generated correctly (sync-request is only used by the getLatestRelease in lib/utils, which is used on the /docs/install page to automatically link to the latest release from GitHub)
Within the docs we link to `/prototype-admin/download-latest` which looks up the latest release from GitHub and redirects to the zip download.

On the install page itself, we look up the latest release URL and then link directly to GitHub from the page.

This moves the `/prototype-admin/download-latest` to `/docs/install` so that it can live within the docs folder and unifies the two approaches so that we consistently use the `/docs/download` URL. This means that we are no longer blocking on a GitHub request before we can render the install page – and that we only make the request to GitHub if the user actually tries to download the kit.
- Rather than trying to extract the tag name from the zipball URL, use the value of `name` that we get in the response data
- Use template literals to build the zip URL rather than multiple variables and concatenation
- Avoid assigning things to variables that aren't used more than once (githubUrl, options) which also reduces the amount of seeking you have to do through the code
- Remove url, zipUrl variables by assigning to releaseUrl directly and `return`ing from every scenario
Simplify download latest release logic
This fixes an issue when deploying to CloudFoundry instances (such as GOV.UK PaaS) where the buildpack fails on the semver range:

```
   -----> Nodejs Buildpack version 1.6.28
   -----> Installing binaries
          engines.node (package.json): >=8.9.1 <9.0
          engines.npm (package.json): unspecified (use default)
          **WARNING** Dangerous semver range (>) in engines.node. See: http://docs.cloudfoundry.org/buildpacks/node/node-tips.html
          **ERROR** Unable to install node: improper constraint: >=8.9.1 <9.0
   Failed to compile droplet: Failed to run all supply scripts: exit status 14
```

Using ^8.9.1 instead which is semantically equivalent (any version greater than 8.9.1 but less than 9.0) allows the app to deploy successfully:

```
   -----> Nodejs Buildpack version 1.6.28
   -----> Installing binaries
          engines.node (package.json): ^8.9.1
          engines.npm (package.json): unspecified (use default)
   -----> Installing node 8.11.3
          Download [https://buildpacks.cloudfoundry.org/dependencies/node/node-8.11.3-linux-x64-34b80d71.tgz]
```
joelanman and others added 29 commits January 2, 2019 15:19
gulp-util is deprecated - replace it, following the guidelines at
https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5

We only used this for logging and colouring our log outputs.
It seems there is a problem with npm installing 'peer dependencies'
npm's warning says that we need to install this dependency ourselves.
The bug is tracked here:
eslint/eslint#11018
Add acorn dependency to fix npm warning
We dont really need it but its needed for Standard and gulp-sourcemaps.

Heroku removes dev dependency by default.
Replace ‘check answers’ pattern with updated code
Update GOV.UK Frontend to version 2.6.0
@fofr fofr merged commit ff9d8a0 into master Feb 4, 2019
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.