Set ignore file to .crxignore like .gitignore #49
Conversation
| @@ -187,6 +188,8 @@ ChromeExtension.prototype = { | |||
| throw new Error('crx.load needs to be called first in order to prepare the workspace.'); | |||
| } | |||
|
|
|||
| var crxignore = gitignoreParser.compile(fs.readFileSync(selfie.path + '/.crxignore', 'utf8')) | |||
There was a problem hiding this comment.
What if the file does not exist?
There was a problem hiding this comment.
Oh, it will be error. I'll fix it. 😢
| @@ -187,6 +188,11 @@ ChromeExtension.prototype = { | |||
| throw new Error('crx.load needs to be called first in order to prepare the workspace.'); | |||
| } | |||
|
|
|||
| var crxignore = null | |||
| try { | |||
| crxignore = gitignoreParser.compile(fs.readFileSync(selfie.path + '/.crxignore', 'utf8')) | |||
There was a problem hiding this comment.
Also, we do not want silent try catch, nor sync operations.
Best would be:
- feeding ignore patterns from the CLI (eg:
crx --ignore */*.js --ignore **/.*... ) - fetching the ignore rules form a
.crxignorefile indeed (but you want the result to be a list of ignored files) (eg:crx --ignore-file .crxignore --ignore-file another/.crxignore)
Both would be provided as a parameter to the class itself.
I reckon this PR addresses the --ignore-file only but you get the idea.
|
Also, because this is a new feature, you want to provide tests to make sure the various edge cases are addressed. And documentation in the README :-) If you never did any of these in the past, no worries, you can ask as many questions as needed to reach a successful merge :-) |
|
@oncletom Thank you for your friendly comments 😸 I think
Yeah, I want to documentation and provide tests. |
|
Cool! Also have a look at https://github.com/jonschlinkert/glob-fs-gitignore or https://www.npmjs.com/package/gitignore-to-glob + node-glob, as they seem to do the job and we're interested in using glob-fs so as grunt-crx can proxy its ignore pattern to crx safely :-) |
|
jonschlinkert/glob-fs-gitignore looks good! |
|
Hey @pastak, although I feel ignore files or selecting them in a finer way (cf. #66) would help, I don't believe supporting an ignore file is the first step to implement it. I'd rather have a good and solid API then add an ignore file if this is a common need. I'd rather stay close to the CLI rather than having an implicit configuration file – we already have too many of them. I hope you don't mind me closing your proposal. |
It allows to build crx and zip without files written into
.crxignore. It can remove useless file like npm modules, README so it makes file size as low as possible.Syntax of
.crxignoreis same as.gitignore. You can write below