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

feat: add verify flag to bypass --no-verify #1494

Closed
wants to merge 1 commit into from
Closed

Conversation

rufman
Copy link

@rufman rufman commented Jul 9, 2018

Description

Some setups (e.g. Gerrit) require commit hooks that add a change id.

So that lerna publish can properly push the commit in these
environments, the --no-verify commit flag should not be specified.
This adds a new flag verify, that if set to true will run the commit
command without thta option, so that all hooks are run.

Motivation and Context

lerna publish does not work in Gerrit environments

How Has This Been Tested?

  • Tested publish in our Gerrit environment with --verify option
  • Added unit tests for option

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@rufman rufman force-pushed the 2.x branch 2 times, most recently from d14f738 to 2003929 Compare Jul 9, 2018
@@ -526,6 +526,14 @@ and you are not on the branch `master` lerna will prevent you from publishing. T
$ lerna publish --allow-branch my-new-feature
```

#### --verify
Copy link

@hutson hutson Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest going with a name like --git-verify as the term --verify is too generic.

const args = ["commit", "--no-verify"];
const args = ["commit"];

if (opts && opts.noVerify) {
Copy link

@hutson hutson Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this default to --no-verify, and only remove --no-verify if opts.verify is set?

Copy link

@hutson hutson Jul 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind. I see how you set noVerify below with the check for verify.

Some setups (e.g. Gerrit) require commit hooks that add a change id.

So that lerna publish can properly push the commit in these
environments, the `--no-verify` commit flag should not be specified.
This adds a new flag `git-verify`, that if set to `true` will run the
commit command without that option, so that all hooks are run.
evocateur added a commit that referenced this issue Jul 27, 2018
BREAKING CHANGE:
* `--preid` now defaults to "alpha" during prereleases:

  The previous default for this option was undefined, which led to an awkward "1.0.1-0" result when passed to `semver.inc()`.

  The new default "alpha" yields a much more useful "1.0.1-alpha.0" result. Any previous prerelease ID will be preserved, just as it was before.

* `--no-verify` is no longer passed to `git commit` by default, but controlled by the new `--commit-hooks` option:

  The previous behavior was too overzealous, and the new option operates exactly like the corresponding [npm version](https://docs.npmjs.com/cli/version#commit-hooks) option of the same name.

  As long as your pre-commit hooks are properly scoped to ignore changes in package.json files, this change should not affect you. If that is not the case, you may pass `--no-commit-hooks` to restore the previous behavior.

Fixes #277
Fixes #936
Fixes #956
Fixes #961
Fixes #1056
Fixes #1118
Fixes #1385
Fixes #1483
Fixes #1494
nicolo-ribaudo pushed a commit to babel/lerna that referenced this issue Dec 18, 2018
BREAKING CHANGE:
* `--preid` now defaults to "alpha" during prereleases:

  The previous default for this option was undefined, which led to an awkward "1.0.1-0" result when passed to `semver.inc()`.

  The new default "alpha" yields a much more useful "1.0.1-alpha.0" result. Any previous prerelease ID will be preserved, just as it was before.

* `--no-verify` is no longer passed to `git commit` by default, but controlled by the new `--commit-hooks` option:

  The previous behavior was too overzealous, and the new option operates exactly like the corresponding [npm version](https://docs.npmjs.com/cli/version#commit-hooks) option of the same name.

  As long as your pre-commit hooks are properly scoped to ignore changes in package.json files, this change should not affect you. If that is not the case, you may pass `--no-commit-hooks` to restore the previous behavior.

Fixes lerna#277
Fixes lerna#936
Fixes lerna#956
Fixes lerna#961
Fixes lerna#1056
Fixes lerna#1118
Fixes lerna#1385
Fixes lerna#1483
Fixes lerna#1494
@lock
Copy link

@lock lock bot commented Dec 27, 2018

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants