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

Improve and simplify Gulp #2717

Closed
Tracked by #1389
domoscargin opened this issue Jul 20, 2022 · 3 comments
Closed
Tracked by #1389

Improve and simplify Gulp #2717

domoscargin opened this issue Jul 20, 2022 · 3 comments
Labels
epic Epics are used in planning project boards to group related stories tooling

Comments

@domoscargin
Copy link
Contributor

domoscargin commented Jul 20, 2022

What

Simplify our use of Gulp by reducing its use to task running. Write our own scripts for the actual tasks, which do not rely on Gulp.

Why

Our gulp tasks are complicated and doing too many things: #2243

Additionally, we cannot update a lot of dependencies because of our reliance on oldie: #2469

We had previously made the decision to move away from Gulp and write our own build scripts.

We essentially removed all Gulp usage, then changed our mind with some exceptions

Composed build tasks can use Gulp

  1. Build targets in tasks/build/** for ./dist and ./package
  2. Review app builds in app/tasks/build/** for ./app/dist

But since every Gulp task such as npx gulp build:package has a corresponding npm run build:package we chose not to document these in tasks.md

Composed build tasks need logging

But then we realised we lost logging in the terminal so brought some Gulp usage back:

  • Limiting gulp.parallel() and gulp.series() for composed build tasks only
  • Limiting gulp.task() usage to gulpfile.mjs configs only

Shared tasks without Gulp

But last of all, we split out all the Review app tasks in #3384 which gave us the opportunity to make all shared tasks work without Gulp. We switched to the export your tasks pattern vs using gulp.task()

Reminder: This API isn't the recommended pattern anymore - export your tasks.

All our new tasks are now plain async functions:

await files.copy('**/*', {
  srcPath: join(paths.src, 'govuk/assets'),
  destPath: join(paths.app, 'dist/assets')
})

Yet they can still be composed into tasks for Gulp's gulp.series() and gulp.parallel() etc

task.name('copy:assets', () => files.copy('**/*', {
  srcPath: join(paths.src, 'govuk/assets'),
  destPath: join(paths.app, 'dist/assets')
}))

Who needs to work on this

Developers

Who needs to review this

Developers

Done when

@domoscargin domoscargin added epic Epics are used in planning project boards to group related stories tooling javascript squad labels Jul 20, 2022
@domoscargin domoscargin changed the title Remove Gulp Remove or improve Gulp Nov 21, 2022
@domoscargin
Copy link
Contributor Author

@colinrotherham

Is there any undocumented work which might be outstanding here? Or should we go ahead and close this sucker?

@colinrotherham
Copy link
Contributor

@domoscargin I think we're still missing those lost edits that captured why we're keeping Gulp

We essentially removed all Gulp usage, then changed our mind with some exceptions

Composed build tasks can use Gulp

  1. Build targets in tasks/build/** for ./dist and ./package
  2. Review app builds in app/tasks/build/** for ./app/dist

But since every Gulp task such as npx gulp build:package has a corresponding npm run build:package we chose not to document these in tasks.md

Composed build tasks need logging

But then we realised we lost logging in the terminal so brought some Gulp usage back:

  • Limiting gulp.parallel() and gulp.series() for composed build tasks only
  • Limiting gulp.task() usage to gulpfile.mjs configs only

Shared tasks without Gulp

But last of all, we split out all the Review app tasks in #3384 which gave us the opportunity to make all shared tasks work without Gulp. We switched to the export your tasks pattern vs using gulp.task()

Reminder: This API isn't the recommended pattern anymore - export your tasks.

All our new tasks are now plain async functions:

await files.copy('**/*', {
  srcPath: join(paths.src, 'govuk/assets'),
  destPath: join(paths.app, 'dist/assets')
})

Yet they can still be composed into tasks for Gulp's gulp.series() and gulp.parallel() etc

task.name('copy:assets', () => files.copy('**/*', {
  srcPath: join(paths.src, 'govuk/assets'),
  destPath: join(paths.app, 'dist/assets')
}))

@domoscargin domoscargin changed the title Remove or improve Gulp Improve and simplify Gulp Apr 12, 2023
@domoscargin
Copy link
Contributor Author

I've updated the description to reflect the changes to our approach.

And with that, I'm going to call this one done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Epics are used in planning project boards to group related stories tooling
Projects
None yet
Development

No branches or pull requests

3 participants