-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement .crxignore #51
Conversation
@augmt Good work 👍 ( I have not worked it for busy sorry... 🙇 ) |
Hey guys, 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. |
This a very useful features especially when your building on transpiled environment, so I have kept a mirror of this commit if anyone wants to install it via. npm: https://github.com/bluejamesbond/crx (credit provided @pastak @augmt). |
@bluejamesbond you are right – could you eventually just implement something which helps configure the following ignore list, from the API and CLI point of view? https://github.com/oncletom/crx/blob/d65dc24d340078e48f918b2ebff44b580a40291a/src/crx.js#L209 Do you think it would it provide the same level of feature as the ignore file? |
In my opinion, a crxignore file is just a simpler. It provides a way to easily maintain the codebase. If a new folder is added, the workflow is quite easy. I just update all my ignore files. In the event we opt for a CLI, then either I have an extra script file which contains the command with all the ignore files appended or I have it in package.json. Updating a script and/or the package.json isn't as easy. The crxignore also allows for nested ignores i.e. if I have a module shared across multiple extensions. And of course, this isn't to say a API/CLI option is not need but that it should be coupled with the crxignore. |
Oh interesting use case! What is the layout of your repos/code then? To me, having a configurable exclusion/files whitelisting at the API level is more important because then it is just a matter of how you feed them (via CLI, via ignore file, via env variables etc.) so I suggest to do it in 3 moves:
Although we have to be careful because the gitignore spec works differently than the minimatch one. @bluejamesbond because the branch diverged, would you be happy to (re)submit any of the above points as a PR? |
This pull request builds upon #49:
.crxignore
is read asynchronously; there is an appropriate CLI command; there is a new test in place; and the documentation has been updated.If it's preferable I can submit the pull request to pastak/crx so that @pastak may then submit their pull request.