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

Travis: Simplify and clean up gulp invocation #19

Merged
merged 1 commit into from
Apr 13, 2018

Conversation

Krinkle
Copy link
Contributor

@Krinkle Krinkle commented Mar 29, 2018

The default 'script' for javascript projects on Travis CI is
'npm test'. This is the preferred entry point, as it will also
be what (future) contributors intuitively try by default after
cloning the repository (npm it; or: npm install && npm test).

The good news is, running 'npm test' on a clone of this repo
already worked by default, because 'npm test' was already set
up in package.json with the 'scripts/test' property to run Gulp.

Note that this change, while not introducing it, has the side-effect
of restoring use of --verbose for the Travis CI build logs.

The even better news is that you don't need npm install -g gulp, because Gulp is already a devDependency which are installed by running npm install.

Now granted, when running npm install, Gulp will only be installed locally, not globally. But, all npm script contexts use the local node_modules/.bin by default in their PATH (which is documented,
stable and intended behaviour). As such, 'gulp' works fine from `npm test.

I won't change the README.md documentation, as it is still useful to have Gulp installed globally in order to conveniently run other gulp tasks, but at least for the minimum of running the tests
from npm test, and for the purposes of Travis CI, it does not need to be installed via .travis.yml.

This also has the benefit of making Travis CI actually use the version of Gulp specified in package.json, instead of the variable latest.

I've also changed npm test from gulp travis --verbose to gulp --verbose because it seems like this was an unintentional remnant. Given that Travis CI was previously not using npm test, this command was not used by Travis. And the default==travis anyway in the current Gulpfile, but it seems better to invoke it through the default than through travis, given this will be used by both travis and locally during development. If in the future such variation is needed again, it is typically recommended to make the vary within the Gulpfile (e.g. change the default based on process.env.CI, or process.env.TRAVIS [1]).

[1] https://docs.travis-ci.com/user/environment-variables/

The default 'script' for javascript projects on Travis CI is
'npm test'. This is the preferred entry point, as it will also
be what (future) contributors intuitively try by default after
cloning the repository (npm it; or: npm install && npm test).

The good news is, running 'npm test' on a clone of this repo
already worked by default, because 'npm test' was already set
up in package.json with the 'scripts/test' property to run Gulp.

Note that this change, while not introducing it, has the side-effect
of restoring use of --verbose for the Travis CI build logs.
@nicjansma
Copy link
Owner

Awesome!

@nicjansma nicjansma merged commit 5100574 into nicjansma:master Apr 13, 2018
@Krinkle Krinkle deleted the build-simple branch April 13, 2018 14:51
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.

None yet

2 participants