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

Simplify gulp by removing separate prod/dev scripts #854

Closed
wants to merge 1 commit into from

Conversation

simison
Copy link
Contributor

@simison simison commented Nov 18, 2018

Work in progress.

The idea is that instead of:

npm run build:prod
npm run build

we would have:

NODE_ENV=production npm run build
npm run build # defaulting to "development"

and the same for:

NODE_ENV=production npm run start
npm run start # defaulting to "development"

It's just a bit annoying how tests are currently set up with their own NODE_ENV=test env so this will need some further work to play nicely. Might be I can chop this into two smaller PRs: tests and the rest.

Testing

  • Scripts should default to NODE_ENV=development if none is set
  • npm start and npm build should produce different builds with NODE_ENV=production and NODE_ENV=development
  • Different npm run test:* scripts should work and not wipe out development database

@simison simison added the build label Nov 18, 2018
@simison simison force-pushed the update/simplify-gulp branch from fd73763 to b977c2b Compare November 18, 2018 23:21
@@ -32,17 +32,19 @@ var environmentAssets,

/**
* Load config + assets
* Note that loading config before `env:*`
* tasks would load configs with wrong environment
*/
function loadConfig(done) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This task should be removed.

@simison simison requested a review from mrkvon November 19, 2018 07:44
simison referenced this pull request Dec 4, 2018
@mrkvon
Copy link
Contributor

mrkvon commented Dec 6, 2018

Is this still a work in progress? Is it ready for a review?

@simison
Copy link
Contributor Author

simison commented Dec 6, 2018

I'd say it's 80% done and up for grabs if you would like to get familiar with it. :-) Gulp is quite simple.

The problem is that our tests expect NODE_ENV=test and manually switch to development environment when running the test. That's fine though, we don't need to run tests in NODE_ENV=production — just might require shuffling some bits and bobs around to make everything run smoothly. :-)

@nicksellen nicksellen mentioned this pull request Dec 13, 2019
5 tasks
@github-actions
Copy link

This pull request is marked as unloved because it has not had any activity for 180 days.

It doesn't mean it's not important, so please remove the unloved label if you like it, or add a comment saying what it means to you :)

However, if you just leave it like this, I'll close it in 14 days to help keep your pull requests tidy!

Thanks!

@github-actions github-actions bot added the Stale label May 30, 2020
@github-actions github-actions bot closed this Jun 14, 2020
@chmac chmac deleted the update/simplify-gulp branch January 29, 2024 18:34
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.

2 participants