add the --verify option #669
Conversation
| pkgPath = path.join(env.configBase, pkgPath); | ||
| } | ||
| gutil.log('Verifying plugins in ' + pkgPath);< 8000 /span> | ||
| return verifyDeps(require(pkgPath).devDependencies || {}); |
There was a problem hiding this comment.
Too much nesting, use vars.
shouldn't it also check dependencies as well? verify(Object.keys(devDeps).concat(Object.keys(deps))) makes sense
There was a problem hiding this comment.
Yeh, I was wondering but I though that we don't want to promote the situation where people put gulp plugins as dependencies. Unless there is a legitimate use-case for declaring gulp plugins as dependencies instead of devDependencies?
There was a problem hiding this comment.
@pkozlowski-opensource It isn't promoting it, just supporting. --verify should support all deps, normal/peer/dev otherwise we are going to get hosed with "bugs" like "--verify does not works"
|
To answer the question of whether blackList.json should be an npm module: I don't think it should be due to it being constantly influx. If it were in npm, it would need to be versioned and I don't think applying semver to it makes sense. |
| @@ -68,6 +70,15 @@ function handleArguments(env) { | |||
| process.exit(0); | |||
| } | |||
|
|
|||
| if (opts.verify) { | |||
| var pkgPath = opts.verify !== true ? opts.verify : 'package.json'; | |||
| if (path.resolve(pkgPath) !== path.normalize(pkgPath)) { | |||
There was a problem hiding this comment.
I think there are probably modules for finding the package.json that may be more verbose than this.
|
Can't we just use https://github.com/tkellen/node-matchdep for this? /cc @tkellen |
|
@phated nice find - I think |
|
@contra couldn't you just use the blacklist array as the filterAll glob and then log the results? |
|
@phated also makes sense |
|
Wohaaa, this is awesome - this is what I love about open source and people who care about code quality! Thank you guys, I'm going to work through all the comments. |
👍 |
|
|
||
| /** | ||
| * Given a collection of plugin names verifies this collection against | ||
| * the black-list. Invokes callback with an object: |
|
👍 rad feature |
|
I'm going to merge this and clean it up since I am working on gulp4 today and want to rebase. Thanks @pkozlowski-opensource, always love when you jump in. |
|
Cleaned up in 39ad456 |
|
More in f32c172 - keeping the logging and fetching in the bin make it easier to test the parts that matter. |
This is the second attempt at #535 - this time targeting the gulp 4 branch. I think I've tackled all the comments left in the original PR (#668)