Add a --no-verify option to prevent git hooks from being verified

[noahrc]

Thanks for the awesome tool! We use a git pre-commit hook to test our code before committing and it seems to prevent the last step of standard-version (git tag). This PR adds a --no-verify option to disable git hook verification.

[bcoe]

@noahrc this seems great to me, what do you think @stevemao?

[stevemao]

Depends on how you see the scope of this module. I personally think it should be consistent with npm-version but looks like it doesn't support this.

But I like this feature. Maybe we could add it to npm-version?

[bcoe]

@stevemao I'll mention the idea to @othiym23 and team, seems like it could be a useful option.

Having said that, I think this is a pretty nice little addition to standard-version, and would help get another team using our tool for their release flow ... my temptation is to add this.

[bcoe]

@noahrc mind rebasing with master?

[nexdrew]

@noahrc One small nit from me - can we document the --no-verify option to the CLI help text (by adding the .option() to yargs)?

@bcoe @stevemao Thoughts?

[nexdrew]

This is great, nice work!

[noahrc]

Thanks, all. I rebased off master and added some CLI help text for no-verify.

[stevemao]

nit: can you remove the full-stop?

[nexdrew]

@stevemao Not sure what you mean. Do you want Noah to remove the period at the end of the description?

[stevemao]

Yes, just to be consistent.

[nexdrew]

Ok gotcha, that makes sense. Thanks for clarifying. 👍

[nexdrew]

@noahrc Because of the way that yargs (actually yargs-parser) handles boolean flags by interpreting --no-anything to just negate argv.anything without setting argv.noAnything (or its aliases) to true, your -n alias actually doesn't work as is. But I agree that we should keep --no-verify with an -n alias to be consistent with the git commit flags.

To reconcile the difference, I think we should add a check for argv.n to your logic, changing this:

var verify = argv.verify === false ? '--no-verify ' : ''

to this:

var verify = argv.verify === false || argv.n ? '--no-verify ' : ''

According to my tests, that produces the desired behavior.

$ ./index.js
OLD: use --no-verify? false
NEW: use --no-verify? false

$ ./index.js --no-verify
OLD: use --no-verify? true
NEW: use --no-verify? true

$ ./index.js -n
OLD: use --no-verify? false
NEW: use --no-verify? true

[noahrc]

Interesting, thanks for the explanation of how yargs works. I'll make that change and add a test for '-n'.

[stevemao]

Because of the way that yargs (actually yargs-parser) handles boolean flags by interpreting --no-anything to just negate argv.anything without setting argv.noAnything (or its aliases) to true

Hmm... A bit counterintuitive and nowadays I tend not to do too much magic to modules. But since it's already doing it, maybe its worth to fix it in yargs-parser? @bcoe

[noahrc]

I went ahead and added the argv.n check, but can revert if yargs-parser changes.

[nexdrew]

I totally forgot about the ability to disable boolean negation in yargs-parser. We could apply the following config to package.json:

  "yargs": {
    "boolean-negation": false
  }

But we'd still have to check for argv.n or argv.noVerify to be true (instead of checking for argv.verify to be false).

Either way, this will now work as is and looks shippable to me. I vote merge. 😎

[coveralls]

Coverage increased (+0.06%) to 95.294% when pulling b37bc84 on noahrc:master into 178e001 on conventional-changelog:master.

[bcoe]

@noahrc give this a shot:

npm i standard-version@next

thanks a ton for the contribution.

[noahrc]

works for me!

Thanks for making great open source tools 😀