Specify which areas of package to upgrade #16
Conversation
There was a problem hiding this comment.
Thanks! A few comments.
Also, please make sure that the main salita function throws if the sections option is invalid, and also please ensure that bin/salita exits with an argument error if you:
- pass duplicate section names
- pass a section name that we do not support
- pass the argument, but no section names
README.md
Outdated
| @@ -27,6 +27,7 @@ salita | |||
| - `--ignore-stars`: ignore updates to packages that are set to "*" | |||
| - `--ignore-pegged`: ignore updates to packages that are pegged to a single version, rather than a range | |||
| - `--check`: implies "dry-run"; and returns with an exit code matching the number of updated dependencies. | |||
| - `--sections`: comma-separated list of sections to process, default: dep,dev,pee,opt. | |||
There was a problem hiding this comment.
s/pee/peer/g, there's no need to make them all be 3 letters - and "pee" isn't really a good word choice :-)
bin/salita
Outdated
| @@ -27,6 +27,8 @@ var options = require('yargs') | |||
| .boolean('ignore-pegged').describe('ignore-pegged', 'ignore updates to packages that are pegged to a single version, rather than a range') | |||
| .boolean('check').describe('check', 'implies --dry-run and --no-update, and returns with an exit code matching the number of updated dependencies') | |||
| .default('check', FALSE).alias('check', 'c') | |||
| .string('sections').describe('sections', 'comma-separated list of sections to process, default: dep,dev,pee,opt') | |||
There was a problem hiding this comment.
i believe this can be made an array instead of a string, and yargs should do the splitting for us?
index.js
Outdated
| var sectionCheck = function (options, key) { | ||
| return options.sections.some(function (section) { | ||
| var length = section.length; | ||
| return length > 2 && key.slice(0, length) === section; |
There was a problem hiding this comment.
Rather than an implicit relationship, let's please use an explicit map of option → section name.
index.js
Outdated
| var depLookup = Promise.all(dependenciesLookup(pkg.data, key, options['ignore-stars'], options['ignore-pegged'])); | ||
| depLookups.push(depLookup); | ||
| depPromises.push(depLookup.then(options.json ? createResultJSON(key) : createResultTable(title))); | ||
| if (sectionCheck(options, key)) { |
There was a problem hiding this comment.
this can probably just pass options.section; there's no need to pass the entire object here.
bin/salita
Outdated
| return false; | ||
| }; | ||
|
|
||
| var sections = ['dep', 'dev', 'peer', 'opt']; |
There was a problem hiding this comment.
since "opt" isn't actually supported right now, it probably shouldn't be included right now?
Also, it'd probably be better if the main salita function exports this list rather than duplicating it in bin/salita
|
No problem. Glad to help when I can. |
|
@Xotic750 in testing this locally, when i do Also, in using this, I think that |
#15