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 50d0f62f..639e4352 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" }, @@ -102,6 +104,6 @@ }, "engineStrict": true, "engines": { - "node": ">= 0.12" + "node": ">= 4" } } 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/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(); } 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;