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

Install release tools with npm rather than via pre-commit hook #666

Merged
merged 1 commit into from
Aug 2, 2018

Conversation

phillipj
Copy link
Collaborator

In preparation for publish a couple of new versions of mustache.js, I stumbled upon a couple of improvements.

Previously mocha, uglifyjs and jshint used when publishing a new version were ensured / installed by Rakefile via the pre-commit git hook. Those modules were automatically installed globally if not present in $PATH.

Installing modules globally has one particular downside that we should avoid: we can't ensure developers publishing new mustache.js versions have the same version of these 3rd party modules installed.

If instead we make these modules local dev dependencies, we can ensure which versions are installed, and we make 3rd party dependencies explicit by listing them in package.json, rather than in Rakefile.

Previously mocha, uglifyjs and jshint used when publishing a new version
were ensured / installed by `Rakefile` via the pre-commit git hook.
Those modules were automatically installed globally if not present in
`$PATH`.

Installing modules globally has one particular dowsides that we should
avoid: we can't ensure developers publishing new mustache.js versions
have the same version of these 3rd party modules installed.

If instead we make these modules local dev dependencies, we can ensure
which versions are installed, and we make 3rd party depedencies explicit
by listing them in `package.json`, rather than in `Rakefile`.
@phillipj
Copy link
Collaborator Author

@dasilvacontin if you remember any particular reason we've avoided installing these modules via npm before, please share :)

@phillipj phillipj merged commit 0a9999a into janl:master Aug 2, 2018
@phillipj phillipj deleted the install-uglify-with-npm branch August 2, 2018 10:07
phillipj added a commit that referenced this pull request Aug 7, 2018
Previously when replacing dependency installation with `npm` rather than
`rake`, there were still install task references found in `Rakefile`
which should have been removed as well.

```
$ npm version patch
v2.3.1

...

npm ERR! > minifying `mustache.js`...
npm ERR! rake aborted!
npm ERR! Don't know how to build task 'install_uglify'
```

Those `install_*` tasks does not exist anymore, as installing those
dependencies are done with npm now.

Refs #666
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.

1 participant