Ci issues with commits #402
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* master: chore(ci): add node versions and say git version in ci
LinusU
reviewed
Dec 14, 2016
| @@ -11,6 +11,7 @@ function log(repoPath, done) { | |||
| cwd: repoPath | |||
| }, function(error, stdout, stderr) { | |||
| if (error) { | |||
| throw new Error(error); | |||
There was a problem hiding this comment.
Isn't error already an Error-instance? Maybe just throw error?
There was a problem hiding this comment.
also, this makes the line below (done();) unreachable since a throw interrupts the execution of the function
There was a problem hiding this comment.
I agree it already is an instance of Error. I'll change that.
The reason I decided to throw here is that this is actually fatal. If we can't do git log, we can't check if the commit worked and the process needs to throw so that our tests can return the correct error code and output an actual error to stderr. It is only used in test and was silently failing in the previous configuration.
There was a problem hiding this comment.
Also, we could certainly benefit from re-writing the codebase to use (err, cb) style 😸
Instead of creating a new error instance, this uses the existing error instance and just throws it.
jimthedev
added a commit
that referenced
this pull request
Dec 19, 2016
* master: docs(install): Add documentation for installing and running locally (#300) fix(tests): fix tests when a global config is present (#405) test(adapter): add test for scoped npm modules fix(adapter): add support for scoped adapters docs: Add cz-emoji to tools list (#404) feat(commit): use OS-specific cache dir for commitizen.json instead of home-or-tmp (#400) fix(npmignore): ignore more unnecessary files (#393) docs: add vscode-commitizen to tools list (#397) Ci issues with commits (#402) chore(ci): add node versions and say git version in ci docs: replace sudo mention with link to how to fix EACCES error
jimthedev
added a commit
that referenced
this pull request
Jan 2, 2017
* master: fix(package): "main" property within "package.json" (#409) style: Add linting using eslint (#406) ci(appveyor): remove appveyor on finish script ci(appveyor): disable collection of artifacts ci(tests): on windows run tests as node4/npm3 ci(travis): update semantic-release to 6.3.5 and update travis config chore(package): update nyc to version 10.0.0 (#392) docs(install): Add documentation for installing and running locally (#300) fix(tests): fix tests when a global config is present (#405) test(adapter): add test for scoped npm modules fix(adapter): add support for scoped adapters docs: Add cz-emoji to tools list (#404) feat(commit): use OS-specific cache dir for commitizen.json instead of home-or-tmp (#400) fix(npmignore): ignore more unnecessary files (#393) docs: add vscode-commitizen to tools list (#397) Ci issues with commits (#402) chore(ci): add node versions and say git version in ci docs: replace sudo mention with link to how to fix EACCES error
plroebuck
reviewed
Apr 2, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #401.