Adds support for both yarn and npm. #22
Conversation
There was a problem hiding this comment.
Hi @asyed94,
Thank you for submitting a PR! In general, it's good. I left a few comments.
bin/package.boilerplate.json
Outdated
| @@ -1,9 +1,10 @@ | |||
| { | |||
| "devDependencies": { | |||
| "poetic": "^1.0.5" | |||
| "poetic": "^1.0.5", | |||
| "yarpm": "^0.2.1" | |||
There was a problem hiding this comment.
Let's try to keep poetic as a single dependency. Also yarpm doesn't seem to be well maintained.
There was a problem hiding this comment.
Agreed, it would definitely be nicer to have poetic as a single dependency. 👍
bin/install-boilerplate.js
Outdated
| @@ -66,7 +66,11 @@ const updatePackageJson = () => { | |||
| const installPackages = () => { | |||
| try { | |||
| console.log('🍉 Installing packages ...'); | |||
| cp.execSync('yarn install'); | |||
| if (fse.existsSync('yarn.lock')) { | |||
There was a problem hiding this comment.
Thanks! However, after looking at it again with a fresh mind, I don't think this covers all cases.
If the user runs poetic immediately after git init and yarn/npm init their project dir will not contain a yarn.lock file 🙁. A better solution would probably be to try yarn install first, then if that throws an error run npm install.
There was a problem hiding this comment.
Or provide an input similar to how CLI tools like vue-cli do and ask the user what their preference is...
Or provide a CLI argument to specify it... ;)
Honestly it seems odd to default to a dependency that isn't installed by default with node (or installed as a dep with the package in question.
There was a problem hiding this comment.
That's a good point, I agree. It should be customizable. Ideally, it should support a CLI argument, and if that is not provided it should ask what to use.
There was a problem hiding this comment.
@arianacosta when will this be available via npm/npx, it was the one thing preventing me from using this and I am hoping to simplify a bit of boilerplate on some 100% npm driven projects.
There was a problem hiding this comment.
@ktiedt I was planning on releasing it this weekend, unfortunately, this doesn't work. This installs and all, but the functionality is broken. I've created another ticket for it: #28
I hope we can find a solution soon, but having NPM is going to be more challenging than what we originally thought. I'll keep you posted 👍
# [1.2.0](v1.1.0...v1.2.0) (2019-12-02) ### Bug Fixes * removes installation with npm and upgrades dependencies ([b410ffa](b410ffa)) ### Chores * add semantic release workflow ([40e6e47](40e6e47)), closes [#33](#33) ### Continuous Integration * **release:** fix issue with protected branch ([#37](#37)) ([67c8980](67c8980)) ### Documentation * **contributing:** improve instructions for contributing ([35a4696](35a4696)) ### Features * simplify installation script ([#36](#36)) ([ab52404](ab52404)) ### Miscellaneous * Adds support for both yarn and npm. (#22) ([8caa239](8caa239)), closes [#22](#22) * updated readme ([01f2c32](01f2c32))
|
🎉 This PR is included in version 1.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
The first commit makes poetic check for a
yarn.lockfile to decide whether to usenpm installoryarn installand adds yarpm to the included npm scripts for cross compatibility.The second one just adds a
package-lock.jsonto the repo for the npm crowd.