From a0752b4db9bb8a8c482afa90425acd3a881d5a09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Sun, 8 Jan 2017 18:49:05 +0100 Subject: [PATCH 1/6] refactor(git): use execa to invoke git Use the execa library when invoing git --- package.json | 2 + src/cli/strategies/git-cz.js | 14 ++++--- src/commitizen/commit.js | 74 +++++++++++++++--------------------- src/git/add.js | 7 ++-- src/git/commit.js | 29 +++----------- src/git/init.js | 7 ++-- src/git/log.js | 16 +++----- test/tests/commit.js | 50 +++++++++--------------- test/tests/staging.js | 4 +- 9 files changed, 79 insertions(+), 124 deletions(-) diff --git a/package.json b/package.json index 50d0f62f..8080f783 100644 --- a/package.json +++ b/package.json @@ -77,6 +77,7 @@ "cz-conventional-changelog": "1.2.0", "dedent": "0.6.0", "detect-indent": "4.0.0", + "execa": "0.5.1", "find-node-modules": "1.0.4", "find-root": "1.0.0", "fs-extra": "^1.0.0", @@ -85,6 +86,7 @@ "lodash": "4.17.2", "minimist": "1.2.0", "path-exists": "2.1.0", + "pify": "2.3.0", "shelljs": "0.7.5", "strip-json-comments": "2.0.1" }, diff --git a/src/cli/strategies/git-cz.js b/src/cli/strategies/git-cz.js index 7af5f891..60697586 100644 --- a/src/cli/strategies/git-cz.js +++ b/src/cli/strategies/git-cz.js @@ -60,16 +60,20 @@ function gitCz (rawGitArgs, environment, adapterConfig) { let adapterPackageJson = getParsedPackageJsonFromPath(resolvedAdapterRootPath); let cliPackageJson = getParsedPackageJsonFromPath(environment.cliPath); console.log(`cz-cli@${cliPackageJson.version}, ${adapterPackageJson.name}@${adapterPackageJson.version}\n`); - commit(sh, inquirer, process.cwd(), prompter, { + commit(inquirer, process.cwd(), prompter, { args: parsedGitCzArgs, disableAppendPaths: true, emitData: true, quiet: false, retryLastCommit - }, function (error) { - if (error) { - throw error; - } + }).catch(function (error) { + // + // Throw in next tick to use Node.js built in error handling + // + // FIXME: This should probably be refactored so that this + // function returns a rejected promise instead... + // + process.nextTick(function () { throw error; }); }); }); diff --git a/src/commitizen/commit.js b/src/commitizen/commit.js index 4743f803..c5056aa1 100644 --- a/src/commitizen/commit.js +++ b/src/commitizen/commit.js @@ -1,5 +1,6 @@ import path from 'path'; +import pify from 'pify'; import dedent from 'dedent'; import cacheDir from 'cachedir'; import {ensureDir} from 'fs-extra'; @@ -8,62 +9,47 @@ import * as cache from './cache'; export default commit; -/** - * Takes all of the final inputs needed in order to make dispatch a git commit - */ -function dispatchGitCommit (sh, repoPath, template, options, overrideOptions, done) { - // Commit the user input -- side effect that we'll test - gitCommit(sh, repoPath, template, { ...options, ...overrideOptions }, function (error) { - done(error, template); +function askUser (inquirer, prompter) { + return new Promise(function (resolve, reject) { + prompter(inquirer, function (arg0, arg1) { + // Allow adapters to error out by providing an Error + if (arg0 instanceof Error) { + return reject(arg0); + } + + resolve({ template: arg0, overrideOptions: arg1 }); }); + }); } /** * Asynchronously commits files using commitizen */ -function commit (sh, inquirer, repoPath, prompter, options, done) { +function commit (inquirer, repoPath, prompter, options) { var cacheDirectory = cacheDir('commitizen'); var cachePath = path.join(cacheDirectory, 'commitizen.json'); - ensureDir(cacheDirectory, function (error) { - if (error) { - console.error("Couldn't create commitizen cache directory: ", error); - // TODO: properly handle error? - } else { - if (options.retryLastCommit) { - - console.log('Retrying last commit attempt.'); + return pify(ensureDir)(cacheDirectory).then(function () { + if (options.retryLastCommit) { + console.log('Retrying last commit attempt.'); - // We want to use the last commit instead of the current commit, - // so lets override some options using the values from cache. - let { - options: retryOptions, - overrideOptions: retryOverrideOptions, - template: retryTemplate - } = cache.getCacheValueSync(cachePath, repoPath); - dispatchGitCommit(sh, repoPath, retryTemplate, retryOptions, retryOverrideOptions, done); + // We want to use the last commit instead of the current commit, + // so lets override some options using the values from cache. + let { + options: retryOptions, + overrideOptions: retryOverrideOptions, + template: retryTemplate + } = cache.getCacheValueSync(cachePath, repoPath); - } else { - // Get user input -- side effect that is hard to test - prompter(inquirer, function (error, template, overrideOptions) { - // Allow adapters to error out - // (error: Error?, template: String, overrideOptions: Object) - if (!(error instanceof Error)) { - overrideOptions = template; - template = error; - error = null; - } + return gitCommit(repoPath, retryTemplate, { ...retryOptions, ...retryOverrideOptions }); + } - if (error) { - return done(error); - } + // Get user input -- side effect that is hard to test + return askUser(inquirer, prompter).then(function ({ template, overrideOptions }) { + // We don't want to add retries to the cache, only actual commands + cache.setCacheValueSync(cachePath, repoPath, { template, options, overrideOptions }); - // We don't want to add retries to the cache, only actual commands - cache.setCacheValueSync(cachePath, repoPath, { template, options, overrideOptions }); - dispatchGitCommit(sh, repoPath, template, options, overrideOptions, done); - }); - } - } + return gitCommit(repoPath, template, { ...options, ...overrideOptions }); + }); }); - } diff --git a/src/git/add.js b/src/git/add.js index 68b75d7f..7d7769de 100644 --- a/src/git/add.js +++ b/src/git/add.js @@ -1,9 +1,10 @@ +import execa from 'execa'; + export { addPath }; /** * Synchronously adds a path to git staging */ -function addPath (sh, repoPath) { - sh.cd(repoPath); - sh.exec('git add .'); +function addPath (repoPath) { + execa.sync('git', ['add', '.'], { cwd: repoPath }); } diff --git a/src/git/commit.js b/src/git/commit.js index 2a0f8128..7dd09594 100644 --- a/src/git/commit.js +++ b/src/git/commit.js @@ -1,6 +1,7 @@ import os from 'os'; import {spawn} from 'child_process'; +import execa from 'execa'; import dedent from 'dedent'; import {isString} from '../common/util'; @@ -9,29 +10,9 @@ export { commit }; /** * Asynchronously git commit at a given path with a message */ -function commit (sh, repoPath, message, options, done) { - let called = false; - let args = ['commit', '-m', dedent(message), ...(options.args || [])]; - let child = spawn('git', args, { - cwd: repoPath, - stdio: options.quiet ? 'ignore' : 'inherit' - }); +function commit (repoPath, message, options) { + const args = ['commit', '-m', dedent(message), ...(options.args || [])]; + const opts = { cwd: repoPath, stdio: options.quiet ? 'ignore' : 'inherit' }; - child.on('error', function (err) { - if (called) return; - called = true; - - done(err); - }); - - child.on('exit', function (code, signal) { - if (called) return; - called = true; - - if (code) { - done(Object.assign(new Error(`git exited with error code ${code}`), { code, signal })); - } else { - done(null); - } - }); + return execa('git', args, opts); } diff --git a/src/git/init.js b/src/git/init.js index 43b4c1fe..602ef961 100644 --- a/src/git/init.js +++ b/src/git/init.js @@ -1,9 +1,10 @@ +import execa from 'execa'; + export { init }; /** * Synchronously creates a new git repo at a path */ -function init (sh, repoPath) { - sh.cd(repoPath); - sh.exec('git init'); +function init (repoPath) { + execa.sync('git', ['init'], { cwd: repoPath }); } diff --git a/src/git/log.js b/src/git/log.js index e2add57d..71746362 100644 --- a/src/git/log.js +++ b/src/git/log.js @@ -1,18 +1,12 @@ -import { exec } from 'child_process'; +import execa from 'execa'; export { log }; /** * Asynchronously gets the git log output */ -function log (repoPath, done) { - exec('git log', { - maxBuffer: Infinity, - cwd: repoPath - }, function (error, stdout, stderr) { - if (error) { - throw error; - } - done(stdout); - }); +function log (repoPath) { + const opts = { maxBuffer: Infinity, cwd: repoPath }; + + return execa.stdout('git', ['log'], opts); } diff --git a/test/tests/commit.js b/test/tests/commit.js index 21dab70a..e88f8798 100644 --- a/test/tests/commit.js +++ b/test/tests/commit.js @@ -33,7 +33,7 @@ beforeEach(function () { describe('commit', function () { - it('should commit simple messages', function (done) { + it('should commit simple messages', function () { this.timeout(config.maxTimeout); // this could take a while @@ -68,16 +68,12 @@ describe('commit', function () { // Pass in inquirer but it never gets used since we've mocked out a different // version of prompter. - commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}, function () { - log(repoConfig.path, function (logOutput) { - expect(logOutput).to.have.string(dummyCommitMessage); - done(); - }); - }); - + return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}) + .then(() => log(repoConfig.path)) + .then(logOutput => expect(logOutput).to.have.string(dummyCommitMessage)) }); - it('should commit message with quotes', function (done) { + it('should commit message with quotes', function () { this.timeout(config.maxTimeout); // this could take a while @@ -112,17 +108,13 @@ describe('commit', function () { // Pass in inquirer but it never gets used since we've mocked out a different // version of prompter. - commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}, function () { - log(repoConfig.path, function (logOutput) { - expect(logOutput).to.have.string(dummyCommitMessage); - done(); - }); - }); - + return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}) + .then(() => log(repoConfig.path)) + .then(logOutput => expect(logOutput).to.have.string(dummyCommitMessage)) }); - it('should commit multiline messages', function (done) { + it('should commit multiline messages', function () { this.timeout(config.maxTimeout); // this could take a while @@ -173,16 +165,12 @@ describe('commit', function () { // Pass in inquirer but it never gets used since we've mocked out a different // version of prompter. - commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true}, function () { - log(repoConfig.path, function (logOutput) { - expect(logOutput).to.have.string(dummyCommitMessage); - done(); - }); - }); - + return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true}) + .then(() => log(repoConfig.path)) + .then(logOutput => expect(logOutput).to.have.string(dummyCommitMessage)) }); - it('should allow to override git commit options', function (done) { + it('should allow to override git commit options', function () { this.timeout(config.maxTimeout); // this could take a while @@ -222,14 +210,12 @@ describe('commit', function () { // Pass in inquirer but it never gets used since we've mocked out a different // version of prompter. - commitizenCommit(sh, inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}, function () { - log(repoConfig.path, function (logOutput) { + return commitizenCommit(inquirer, repoConfig.path, prompter, {disableAppendPaths: true, quiet: true, emitData: true}) + .then(() => log(repoConfig.path)) + .then(logOutput => { expect(logOutput).to.have.string(author); expect(logOutput).to.have.string(dummyCommitMessage); - done(); }); - }); - }); }); @@ -263,11 +249,11 @@ function quickPrompterSetup (sh, repoConfig, adapterConfig, commitMessage, optio commit(commitMessage, options); } - gitInit(sh, repoConfig.path); + gitInit(repoConfig.path); writeFilesToPath(repoConfig.files, repoConfig.path); - gitAdd(sh, repoConfig.path); + gitAdd(repoConfig.path); // NOTE: In the real world we would not be returning // this we would instead be just making the commented diff --git a/test/tests/staging.js b/test/tests/staging.js index b6642426..89670818 100644 --- a/test/tests/staging.js +++ b/test/tests/staging.js @@ -49,7 +49,7 @@ describe('staging', function () { } }; - gitInit(sh, repoConfig.path); + gitInit(repoConfig.path); staging.isClean('./@this-actually-does-not-exist', function (stagingError) { expect(stagingError).to.be.an.instanceof(Error); @@ -60,7 +60,7 @@ describe('staging', function () { writeFilesToPath(repoConfig.files, repoConfig.path); - gitAdd(sh, repoConfig.path); + gitAdd(repoConfig.path); staging.isClean(repoConfig.path, function (afterWriteStagingIsCleanError, afterWriteStagingIsClean) { expect(afterWriteStagingIsCleanError).to.be.null; From b191795d04caeb77c63654d1186d404588a194d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Sun, 8 Jan 2017 18:50:49 +0100 Subject: [PATCH 2/6] fix(commitizen): fix bug when trying to read stdout On my system `stdout` was undeifned and resulted in "Cannot read property 'trim' of undefined". Switching this code to also use execa fixes that issue. --- src/commitizen/adapter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commitizen/adapter.js b/src/commitizen/adapter.js index ade63f1b..420a0db6 100644 --- a/src/commitizen/adapter.js +++ b/src/commitizen/adapter.js @@ -3,7 +3,7 @@ import fs from 'fs'; import findNodeModules from 'find-node-modules'; import _ from 'lodash'; import detectIndent from 'detect-indent'; -import sh from 'shelljs'; +import execa from 'execa'; import {isFunction} from '../common/util'; @@ -150,5 +150,5 @@ function resolveAdapterPath (inboundAdapterPath) { } function getGitRootPath () { - return sh.exec('git rev-parse --show-toplevel').stdout.trim(); + return execa.sync('git', ['rev-parse', '--show-toplevel']).stdout.trim(); } From d0772db02fa7b05a0b1c16a40a50565167b8e915 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Sun, 8 Jan 2017 19:28:26 +0100 Subject: [PATCH 3/6] chore: drop support for Node.js 0.12 BREAKING CHANGE: Node.js 0.12 is no longer supported --- .travis.yml | 1 - package.json | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 92e36289..396ad45e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,7 +13,6 @@ node_js: - '6' - '5' - '4' -- '0.12' before_install: - git config --global user.name "TravisCI" - git config --global user.email "commitizen@gmail.com" diff --git a/package.json b/package.json index 8080f783..639e4352 100644 --- a/package.json +++ b/package.json @@ -104,6 +104,6 @@ }, "engineStrict": true, "engines": { - "node": ">= 0.12" + "node": ">= 4" } } From 4783f4be5142896e068c2f4f0ad207e8d3d7c03d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Mon, 9 Jan 2017 03:25:11 +0100 Subject: [PATCH 4/6] feat(commit): print deprecation warning when using provided inquirer (#416) Adapters should depend directly on inquirer instead of using the one provided by us. --- src/commitizen/commit.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/commitizen/commit.js b/src/commitizen/commit.js index c5056aa1..31a56997 100644 --- a/src/commitizen/commit.js +++ b/src/commitizen/commit.js @@ -1,4 +1,5 @@ import path from 'path'; +import {deprecate} from 'util'; import pify from 'pify'; import dedent from 'dedent'; @@ -11,7 +12,11 @@ export default commit; function askUser (inquirer, prompter) { return new Promise(function (resolve, reject) { - prompter(inquirer, function (arg0, arg1) { + const decoratedInquirer = { + prompt: deprecate(inquirer.prompt, 'Using the supplied copy of inquirer is depreacted, please depend on inquirer directly').bind(inquirer) + } + + prompter(decoratedInquirer, function (arg0, arg1) { // Allow adapters to error out by providing an Error if (arg0 instanceof Error) { return reject(arg0); From 3a3d10d52060b339b3108b5da61f90ba10b987a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Mon, 9 Jan 2017 03:25:23 +0100 Subject: [PATCH 5/6] refactor(configLoader): remove deprecated czConfig (#417) BREAKING CHANGE: czConfig is no longer read for config --- README.md | 2 +- src/configLoader/getNormalizedConfig.js | 8 -------- test/tests/configLoader.js | 5 ----- 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/README.md b/README.md index f0805df4..70ed9345 100644 --- a/README.md +++ b/README.md @@ -90,7 +90,7 @@ This just tells Commitizen which adapter we actually want our contributors to us * full relative file names * absolute paths. -Please note that in previous version of Commitizen we used czConfig. **czConfig has been deprecated** and you should migrate to the new format before Commitizen 3.0.0. +Please note that in previous version of Commitizen we used czConfig. **czConfig has been removed** from Commitizen 3.0.0. #### Optional: Install and run Commitizen locally diff --git a/src/configLoader/getNormalizedConfig.js b/src/configLoader/getNormalizedConfig.js index 22b495b0..f9e385a9 100644 --- a/src/configLoader/getNormalizedConfig.js +++ b/src/configLoader/getNormalizedConfig.js @@ -11,14 +11,6 @@ function getNormalizedConfig (config, content) { // Use the npm config key, be good citizens if (content.config && content.config.commitizen) { return content.config.commitizen; - } else if (content.czConfig) { // Old method, will be deprecated in 3.0.0 - - // Suppress during test - if (typeof global.it !== 'function') - { - console.error("\n********\nWARNING: This repository's package.json is using czConfig. czConfig will be deprecated in Commitizen 3. \nPlease use this instead:\n{\n \"config\": {\n \"commitizen\": {\n \"path\": \"./path/to/adapter\"\n }\n }\n}\nFor more information, see: http://commitizen.github.io/cz-cli/\n********\n"); - } - return content.czConfig; } } else { // .cz.json or .czrc diff --git a/test/tests/configLoader.js b/test/tests/configLoader.js index 2473cd7f..67168ac4 100644 --- a/test/tests/configLoader.js +++ b/test/tests/configLoader.js @@ -30,12 +30,7 @@ describe('configLoader', function () { } }; - let oldStyleConfig = { - czConfig: 'myOldConfig' - }; - expect(getNormalizedConfig(config, npmStyleConfig)).to.equal('myNpmConfig'); - expect(getNormalizedConfig(config, oldStyleConfig)).to.equal('myOldConfig'); }); From bc2b6cbb871c00e445c5706fb191a8bb7281a59e Mon Sep 17 00:00:00 2001 From: Jim Cummins Date: Mon, 9 Jan 2017 11:32:26 -0600 Subject: [PATCH 6/6] fix(hooks): remove ghooks in favor of husky This replaces ghooks with husky. ghooks is deprecated in favor of husky. Contributors may need to For commitizen developers, you need to do the following: 1. rm -rf .git/hooks 2. rm -rf node_modules 3. run npm install. Closes #410 --- package.json | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 639e4352..9c7129d7 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "commit": "node bin/git-cz", "build": "babel src --out-dir dist", "build:watch": "babel --watch src --out-dir dist", + "precommit": "npm run lint && npm run test && npm run check-coverage", "prepublish": "not-in-install && npm run build || true", "report-coverage": "nyc report --reporter=lcov | codecov", "write-coverage": "nyc report --reporter=lcov", @@ -21,9 +22,6 @@ "config": { "commitizen": { "path": "./node_modules/cz-conventional-changelog" - }, - "ghooks": { - "pre-commit": "npm run lint && npm run test && npm run check-coverage" } }, "homepage": "https://github.com/commitizen/cz-cli", @@ -60,7 +58,7 @@ "eslint-config-standard": "6.2.1", "eslint-plugin-promise": "3.4.0", "eslint-plugin-standard": "2.0.1", - "ghooks": "1.3.2", + "husky": "0.12.0", "in-publish": "^2.0.0", "mocha": "3.1.2", "node-uuid": "1.4.7",