Use Gulp instead of Grunt #284

gemmaleigh · 2016-10-14T08:32:37Z

This PR rebases #282 on top of master and fixes the conflicts.

cc. @robinwhittleton, @joelanman, @edwardhorsford.

Copied from #282 (thanks @htmlandbacon) 💯

This pull request moves from grunt to gulp scripts, this should remove warnings (#89) from the build.

I've kept the functionality of the tasks largely the same, I've split out some tasks a little more where I thought it made sense but happy to take feedback on them, I've also structured this into smaller files.

A couple of things outside the core gulp scripts :

start.js no longer includes the command line version, I had issues with getting this to fully report the output, I've moved this to the package.json file (runs as was) with the npm command
travis ci - this might need a bit more testing, as I struggled to do a real test with it
Procfile - heroku deployment worked fine for me

Be good to get some eyes on this, as it is quite a big change 👏

gemmaleigh · 2016-10-14T09:14:10Z

This isn't copying the documentation stylesheets or assets, the tasks for these need to be added.

Update: I've copied these in 61839a4, a9539af and d806b21.

- Add new tasks to compile sass for the documentation and copy the documentation assets - Also rename the clear task to clean

This has been done so that the links at the bottom of the docs will work when navigating in GitHub. We need to strip these out when rendering these pages.

tvararu · 2016-10-14T13:37:21Z

gulp/copy-assets.js

+var config = require('./config.json')
+
+gulp.task('copy-assets', function () {
+  return gulp.src(['!' + config.paths.assets + 'sass{,/**/*}', config.paths.assets + '/**'])


I would put a newline after the comma here so it doesn't look like one big rambling path, but feel free to ignore my nit.

tvararu · 2016-10-14T13:37:55Z

gulp/copy-assets.js

+})
+
+gulp.task('copy-documentation-assets', function () {
+  return gulp.src(['!' + config.paths.docsAssets + 'sass{,/**/*}', config.paths.docsAssets + '/**'])


Possible newline here as well.

tvararu · 2016-10-14T13:39:01Z

gulp/nodemon.js

+var config = require('./config.json')
+
+gulp.task('server', function () {
+  nodemon({ignore: [config.paths.public + '*', config.paths.public + '*'],


Same path twice?

tvararu · 2016-10-14T13:46:47Z

gulp/tasks.js

+  runSequence(['copy-toolkit',
+                'copy-template-assets',
+                'copy-elements-sass',
+                'copy-template'], done)


Do these all actually need to run sequentially, or can they run concurrently? I think this could be:

gulp.task('copy-govuk-modules', ['copy-toolkit', 'copy-template-assets', 'copy-elements-sass', 'copy-template'])

This would run a lot faster since it would spin up parallel threads for all the separate copying tasks. You can apply this to the other tasks in this file too; I think that with task dependencies (the optional array argument I used above) you shouldn't, in theory, need to ever use runSequence, though sometimes it can mean you have to do:

gulp.task('the-first-thing', foo) gulp.task('a-thing-before-others', ['the-first-thing'], bar) gulp.task('earlierer-thing', ['a-thing-before-others'], baz) gulp.task('earlier-thing', ['earlierer-thing'], qux) gulp.task('thing', ['earlier-thing'], blah)

Which is an edge case that is obviously better suited to runSequence.

These can happen at the same time.

This reverts commit cba3375.

joelanman · 2016-10-17T17:47:19Z

@htmlandbacon I might be misreading, but I think theres something wrong here.

The old start sequence was:

npm start
node start.js
checks for node_modules
removes old .port.tmp if necessary
runs grunt
listens for ctrl c, and deletes .port.tmp

.port.tmp is created if the kit has to use a port other than 3000 (ie multiple kits are running), so when server.js automatically restarts it uses the same port

the new sequence is:

npm start
node start.js && gulp
(this means run node start.js, then if it finishes running with no errors, run gulp)
checks for node_modules
removes old .port.tmp if necessary
listens for ctrl c, and deletes .port.tmp
(node start.js is done at this point and exits, allowing gulp to run)
gulp does all the things

the problem is with nothing listening for ctrl c, when a kit starts on another port, it writes .port.tmp to the filesystem but when it stops, theres nothing to delete that file.

It's not a major problem, but its not really right. is it possible to kick off gulp from start.js as before?

joelanman · 2016-10-17T18:24:13Z

Looks like we can http://stackoverflow.com/a/28493832

htmlandbacon · 2016-10-18T08:01:36Z

I'll take a look, I had a few quirks with running it from within a js file.

joelanman · 2016-10-18T13:38:30Z

I've made start.js run the gulp default task, and updated nodemon to ignore changes in assets, and to stop setting NODE_ENV

- This way we can check for the node_modules folder and warn if it isn’t install - By removing Gulp from start.js (now renamed to prestart.js) we get the full output from Gulp when the app is run.

robinwhittleton · 2016-10-20T11:25:32Z

I’m happy with this.

gemmaleigh added the in progress label Oct 14, 2016

gemmaleigh mentioned this pull request Oct 14, 2016

Moved from grunt to gulp tasks #282

Closed

gemmaleigh changed the title ~~Use Gulp instead of Grunt (duplicate of #282 with rebase).~~ [DO NOT MERGE] Use Gulp instead of Grunt (duplicate of #282 with rebase). Oct 14, 2016

htmlandbacon and others added 2 commits October 14, 2016 10:32

Moved from grunt to gulp tasks

b3bcb98

Get travis to run gulp test to check the app runs

33cb9cb

gemmaleigh force-pushed the htmlandbacon-gulpy branch from 856b076 to 33cb9cb Compare October 14, 2016 09:32

gemmaleigh added 6 commits October 14, 2016 12:44

Rename assets to paths in config

223aaef

Update config paths for assets

0e74ec6

Add a copy task for the documentation assets

61839a4

Compile the documentation sass

a9539af

Add the new tasks to the generate assets task

d806b21

- Add new tasks to compile sass for the documentation and copy the documentation assets - Also rename the clear task to clean

Fix documentation routes, where an extra .md is added

8c35843

This has been done so that the links at the bottom of the docs will work when navigating in GitHub. We need to strip these out when rendering these pages.

gemmaleigh changed the title ~~[DO NOT MERGE] Use Gulp instead of Grunt (duplicate of #282 with rebase).~~ Use Gulp instead of Grunt (duplicate of #282 with rebase). Oct 14, 2016

gemmaleigh changed the title ~~Use Gulp instead of Grunt (duplicate of #282 with rebase).~~ Use Gulp instead of Grunt Oct 14, 2016

tvararu reviewed Oct 14, 2016

View reviewed changes

gemmaleigh added 6 commits October 14, 2016 16:23

Add newline for readability

84f5999

Set nodemon to ignore the public and node_modules folders

9fb7d9d

Remove runSequence for copy tasks

613d8e1

These can happen at the same time.

Update the watch task to copy assets (images and js)

9490e75

Lint gulp tasks using StandardJS

237a3b0

Add a callback to the copy-assets task

cba3375

gemmaleigh force-pushed the htmlandbacon-gulpy branch from cba3375 to 237a3b0 Compare October 17, 2016 15:35

Revert "Add a callback to the copy-assets task"

d28440c

This reverts commit cba3375.

make start.js start everything, update nodemon task

09122d5

joelanman force-pushed the htmlandbacon-gulpy branch from fdfaf15 to 09122d5 Compare October 19, 2016 16:01

joelanman force-pushed the htmlandbacon-gulpy branch from a47a529 to 09122d5 Compare October 20, 2016 09:30

gemmaleigh and others added 4 commits October 20, 2016 10:56

Use prestart to load prestart.js before start runs Gulp

a4f3250

- This way we can check for the node_modules folder and warn if it isn’t install - By removing Gulp from start.js (now renamed to prestart.js) we get the full output from Gulp when the app is run.

move gulp check to package.json, use gulp to remove .port.tmp

28b973b

change node_modules check, fix port.tmp clean up

c68dac1

standard.js fix

d69471e

robinwhittleton merged commit 0101ff1 into master Oct 20, 2016

gemmaleigh removed the in progress label Oct 20, 2016

robinwhittleton deleted the htmlandbacon-gulpy branch October 20, 2016 11:47

gemmaleigh mentioned this pull request Nov 18, 2016

v5.0.0 #312

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Gulp instead of Grunt #284

Use Gulp instead of Grunt #284

gemmaleigh commented Oct 14, 2016 •

edited

Loading

gemmaleigh commented Oct 14, 2016 •

edited

Loading

tvararu Oct 14, 2016

tvararu Oct 14, 2016

tvararu Oct 14, 2016

tvararu Oct 14, 2016

joelanman commented Oct 17, 2016

joelanman commented Oct 17, 2016

htmlandbacon commented Oct 18, 2016

joelanman commented Oct 18, 2016

robinwhittleton commented Oct 20, 2016

Use Gulp instead of Grunt #284

Use Gulp instead of Grunt #284

Conversation

gemmaleigh commented Oct 14, 2016 • edited Loading

gemmaleigh commented Oct 14, 2016 • edited Loading

tvararu Oct 14, 2016

Choose a reason for hiding this comment

tvararu Oct 14, 2016

Choose a reason for hiding this comment

tvararu Oct 14, 2016

Choose a reason for hiding this comment

tvararu Oct 14, 2016

Choose a reason for hiding this comment

joelanman commented Oct 17, 2016

joelanman commented Oct 17, 2016

htmlandbacon commented Oct 18, 2016

joelanman commented Oct 18, 2016

robinwhittleton commented Oct 20, 2016

gemmaleigh commented Oct 14, 2016 •

edited

Loading

gemmaleigh commented Oct 14, 2016 •

edited

Loading