feat(cli): add "--config" option support by lc-soft · Pull Request #261 · conventional-changelog/commitlint

lc-soft · 2018-02-01T03:32:50Z

Description

Add "--config" option support for cli, and add a simple test case, but this test case are not tested, need @marionebl to help me improve this test.

Motivation and Context

#241

Usage examples

// path/to/my/custom/config/commitlint.config.js
module.exports = {

};

echo "your commit message here" | commitlint --config path/to/my/custom/config/commitlint.config.js # pass

How Has This Been Tested?

Please see the above example.

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 change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
- Test if config in parent directories is ignored if configPath is provided (should)
- Test the options.file parameter in core.load tests
All new and existing tests passed.

marionebl

Got some ideas how to make this test green. Also we will want to add some additional tests:

Test if config in parent directories is ignored if configPath is provided (should)
Test the options.file parameter in core.load tests

marionebl · 2018-02-01T09:13:22Z

@commitlint/core/src/load.js

 	});

-	const local = await explorer.load(cwd);
+	const local = await explorer.load(cwd, filepath);


I thing filepath has to be passed as configPath to the cosmiconfig constructor

marionebl · 2018-02-01T09:16:19Z

@commitlint/core/src/load.js

 };

-async function loadConfig(cwd) {
+async function loadConfig(cwd, filepath) {


Reading cosmiconfig docs (yes they are a bit confusing...) this probably should be (untested)

async function loadConfig(cwd, configPath) { const explorer = cosmiconfig('commitlint', { rcExtensions: true, configPath }); const local = await explorer.load(cwd); }

the configPath is option of cosmiconfig? I mistakenly thought you meant load([searchPath, configPath])

lc-soft · 2018-02-01T09:41:13Z

@commitlint/cli/src/cli.test.js

+	const config = 'config/commitlint.config.js';
+	const actual = await cli([], {cwd, config})('foo: bar');
+	t.true(actual.stdout.includes('type must not be one of [foo]'));
+	t.is(actual.code, 1);


The result was correct when I ran the command manually, but the test case did not pass. what happened?

echo "foo: bar" | npx yarn run commitlint --config @commitlint/cli/fixtures/custom-config-path/config/commitlint.config.js

yarn run v1.3.2 $ /home/lc/Projects/commitlint/node_modules/.bin/commitlint --config @commitlint/cli/fixtures/custom-config-path/config/commitlint.config.js ⧗ input: foo: bar ✖ type must not be one of [foo] [type-enum] ✖ found 1 problems, 0 warnings error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

lc-soft · 2018-02-01T09:46:27Z

How to run commitlint/cli test only?

lc-soft · 2018-02-01T10:17:32Z

@commitlint/cli/src/cli.test.js


+test('should work with --config option', async t => {
+	const cwd = await git.bootstrap('fixtures/custom-config-path');
+	const config = 'config/commitlint.config.js';


const file = 'config/commitlint.config.js'; const actual = await cli(['--config', file], {cwd})('foo: bar');

lc-soft · 2018-02-01T10:31:03Z

How to run 1commitlint/cli1 test only?

npx yarn run lerna run test --stream --scope @commitlint/cli

lc-soft · 2018-02-01T10:40:31Z

@marionebl

Test if config in parent directories is ignored if configPath is provided (should)

I added file @commitlint/cli/fixtures/specify-config-file/commitlint.config.js.

Test the options.file parameter in core.load tests

I do not understand this sentence, can you describe in detail?

marionebl · 2018-02-01T11:13:56Z

Test the options.file parameter in core.load tests
I do not understand this sentence, can you describe in detail?

Please add a unit test for the new config in https://github.com/marionebl/commitlint/blob/master/%40commitlint/core/src/load.test.js

lc-soft · 2018-02-02T08:24:58Z

@commitlint/core/src/load.test.js

+	const file = 'config/commitlint.config.js';
+	const cwd = await git.bootstrap('fixtures/specify-config-file');
+	const actual = await load({rules: {foo: 'what'}}, {cwd, file});
+	t.is(actual.rules.foo, 'bar');


Why the path is not /home/lc/Projects/commitlint/@commitlint/fixtures/specify-config-file/config/commitlint.config.js' ?

load › rules should be loaded from specify config file Rejected promise returned by test. Reason: Error { code: 'ENOENT', errno: -2, path: '/home/lc/Projects/commitlint/@commitlint/core/config/commitlint.config.js', syscall: 'open', message: 'ENOENT: no such file or directory, open \'/home/lc/Projects/commitlint/@commitlint/core/config/commitlint.config.js\'', } error Command failed with exit code 1.

This change can work.

- const file = 'fconfig/commitlint.config.js'; + const file = 'fixtures/specify-config-file/config/commitlint.config.js';

I think we have to resolve the configPath in core.load relative to cwd:

const explorer = cosmiconfig('commitlint', { rcExtensions: true, configPath: path.resolve(cwd, configPath) });

Rejected promise returned by test. Reason: TypeError { message: 'Path must be a string. Received undefined', } loadConfig (src/load.js:81:1) exports.default (src/load.js:14:16) Test.<anonymous> (src/load.test.js:189:23)

const explorer = cosmiconfig('commitlint', { rcExtensions: true, - configPath: path.resolve(cwd, configPath) + configPath: configPath ? path.resolve(cwd, configPath) : null });

marionebl · 2018-02-03T11:47:35Z

Thanks for your patience 👍

marionebl reviewed Feb 1, 2018

View reviewed changes

feat(cli): add "--config" option support (#241)

3b49f26

lc-soft commented Feb 1, 2018

View reviewed changes

lc-soft commented Feb 2, 2018

View reviewed changes

test: add case for cli "--config" option

86cd2ff

marionebl merged commit 2c03ec6 into conventional-changelog:master Feb 3, 2018

gabemeola mentioned this pull request May 3, 2018

docs(cli): added --config flag #328

Merged

7 tasks

Conversation

lc-soft commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

Usage examples

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

marionebl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lc-soft commented Feb 1, 2018

Uh oh!

lc-soft Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lc-soft commented Feb 1, 2018

Uh oh!

lc-soft commented Feb 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marionebl commented Feb 1, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marionebl commented Feb 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

lc-soft commented Feb 1, 2018 •

edited

Loading

lc-soft Feb 1, 2018 •

edited

Loading

lc-soft commented Feb 1, 2018 •

edited

Loading

marionebl commented Feb 3, 2018 •

edited

Loading