Feat(#31): Add ignore option #37

hinok · 2017-03-12T23:11:40Z

This commit add functionality to ignore files
based on provided regexps.
in index.js config must be specified as below

// @create-index {"ignore":["/foo.js"]}

gajus · 2017-03-13T07:56:03Z

Thank you for the PR.

This lack documentation, README.md changes.

gajus · 2017-03-13T07:53:02Z

src/utilities/log.js

@@ -2,8 +2,6 @@ import chalk from 'chalk';
 import moment from 'moment';

 export default (...append) => {
-    /* eslint-disable no-console */
+  /* eslint-disable no-console */


Use // eslint-disable-next-line

gajus · 2017-03-13T07:53:42Z

src/utilities/constants.js

@@ -0,0 +1,3 @@
+const CREATE_INDEX_PATTERN = /(?:^|[\n\r]+)\/\/ @create-index\s?({.*})?[\n\r]+/;


export const CREATE_INDEX_PATTERN ...

gajus · 2017-03-13T07:54:12Z

package.json

@@ -48,7 +48,8 @@
    "create-index": "node ./dist/bin/create-index ./src/utilities",
    "lint": "cross-env NODE_ENV=development eslint ./src ./tests",
    "precommit": "npm run test",
-    "test": "npm run build && npm run lint && cross-env NODE_ENV=development mocha --compilers js:babel-register"
+    "test": "npm run build && npm run lint && cross-env NODE_ENV=development mocha --compilers js:babel-register",
+    "test:watch": "cross-env NODE_ENV=development mocha --watch --reporter spec --bail --compilers js:babel-register"


Remove test:watch.

gajus · 2017-03-13T07:56:44Z

test/writeIndex.js

-    writeIndex([path.resolve(fixturesPath, 'mixed')]);
+  it('creates index with config in target directory', () => {
+    const indexFilePath = path.resolve(fixturesPath, 'with-config/index.js');
+    const ignoredExportLine = `export { default as bar } from './bar.js';`; // eslint-disable-line quotes


Use // eslint-disable-next-line

gajus · 2017-03-13T07:57:14Z

test/writeIndex.js

+};
+
+const appendToFile = (filePath, content) => {
+  try {


Whats the reason for swallowing an error?

gajus · 2017-03-13T07:57:19Z

test/writeIndex.js

+
+const removeFileByPath = (filePath) => {
+  // eslint-disable-next-line no-empty
+  try {


Whats the reason for swallowing an error?

I was trying to follow "code style" in the project so if you look here https://github.com/gajus/create-index/pull/37/files#diff-f546cc5537a07f2741198b32e7ef3aa9L17 the same pattern with swallowing an error has been already used. I decided to just copy and use it.

Anyway I will remove them if you want, no problem :)

Oh, thats in the test/. Meh, I am indifferent.

gajus · 2017-03-13T07:59:26Z

src/utilities/readIndexConfig.js

+import hasIndex from './hasIndex';
+import {CREATE_INDEX_PATTERN} from './constants';
+
+const isString = (str) => {


Remove unnecessary helpers. I am all pro declarative programming, though typeof str === 'string' is already achieving the same.

gajus · 2017-03-13T07:59:40Z

src/utilities/readIndexConfig.js

+};
+
+const isEmptyString = (str) => {
+  return str.length === 0;


Remove unnecessary helpers. I am all pro declarative programming, though str.length is already achieving the same.

hinok · 2017-03-13T18:17:47Z

Sorry for so many changes. I applied all requested changes and removed "swallowing errors" from tests. I added also section about ignore in README.md.

gajus · 2017-03-13T18:34:57Z

Is this ready to be merged?

- Add functionality to ignore files based on provided regular expressions - Support writeIndex and writeIndexCli - in index.js config must be specified as below // @create-index {"ignore":["/foo.js"]}

hinok · 2017-03-13T20:32:18Z

I thought that writeIndexCli.js uses writeIndex.js, it doesn't - my fault, I didn't check it before but it's fixed already.

I also checked this PR on my local project where I use create-index heavily to maintain icons, it works 💃

Please feel free to correct any english language issues.
I think that it's ready to be merged.

gajus · 2017-03-13T20:42:53Z

Thank you

gajus requested changes Mar 13, 2017

View reviewed changes

hinok force-pushed the feature/add-ignore-option branch 3 times, most recently from 5e170c1 to b162c74 Compare March 13, 2017 18:14

Feat(gajus#31): Add ignore option

2e2e7eb

- Add functionality to ignore files based on provided regular expressions - Support writeIndex and writeIndexCli - in index.js config must be specified as below // @create-index {"ignore":["/foo.js"]}

hinok force-pushed the feature/add-ignore-option branch from b162c74 to 2e2e7eb Compare March 13, 2017 20:23

gajus merged commit bf45925 into gajus:master Mar 13, 2017

hinok deleted the feature/add-ignore-option branch March 13, 2017 20:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(#31): Add ignore option #37

Feat(#31): Add ignore option #37

hinok commented Mar 12, 2017

gajus commented Mar 13, 2017

gajus Mar 13, 2017

gajus Mar 13, 2017

gajus Mar 13, 2017

gajus Mar 13, 2017

gajus Mar 13, 2017

gajus Mar 13, 2017

hinok Mar 13, 2017

gajus Mar 13, 2017

gajus Mar 13, 2017

gajus Mar 13, 2017

hinok commented Mar 13, 2017

gajus commented Mar 13, 2017

hinok commented Mar 13, 2017 •

edited

Loading

gajus commented Mar 13, 2017

		@@ -0,0 +1,3 @@
		const CREATE_INDEX_PATTERN = /(?:^\|[\n\r]+)\/\/ @create-index\s?({.*})?[\n\r]+/;

Feat(#31): Add ignore option #37

Feat(#31): Add ignore option #37

Conversation

hinok commented Mar 12, 2017

gajus commented Mar 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hinok commented Mar 13, 2017

gajus commented Mar 13, 2017

hinok commented Mar 13, 2017 • edited Loading

gajus commented Mar 13, 2017

hinok commented Mar 13, 2017 •

edited

Loading