Skip to content
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

Ignore commonly unwanted files when building XPI #130

Merged
merged 1 commit into from
Mar 22, 2016

Conversation

kumar303
Copy link
Contributor

@kumar303 kumar303 commented Mar 9, 2016

Fixes #85

filesToIgnore: Array<string>;

constructor({filesToIgnore}: Object = {}) {
this.filesToIgnore = filesToIgnore || [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks great!

The only thing that I'm not completely sure about is related to the filesToIgnore defaults and how its overriding will work (especially when it will be available through a config file or a command line option), and as this feature is not exposed to the user yet, most of this could be discussed and resolved in follow-ups if we prefer.

here are a couple of doubts/ideas:

  • by looking into the test cases I though that manifest.json is not a great example of possible filesToIgnore customization, because we probably want to be sure it is never excluded by mistake (e.g. by always prepending "!manifest.json" to this array of patterns) or the resulting xpi could be invalid even if the extension source dir was completely valid (excluding the hypotetic filesToIgnore customization).
  • how about excluding any hidden file by default? (e.g. by using "**/.*" instead of "**/.git"
  • if we suppose that the addon developer can customize the excluded/included patterns somehow, how about composing the default patterns and the user patterns instead of a providing a full override by default? e.g. something like [].concat(mandatory_patterns, user_patterns, defaults_patterns)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points! I fixed the first two. As for concatenating filter overrides, I left it as is. I think this will become more clear once we implement customizations -- I don't want to make it too flexible before then. The feature I added to override all default filter rules will probably still be necessary since it is valid to include nested XPIs inside an XPI. Some very large add-ons do this.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 98.947% when pulling 9ce77c4 on kumar303:build-filter into 01ed902 on mozilla:master.

kumar303 added a commit that referenced this pull request Mar 22, 2016
Ignore commonly unwanted files when building XPI
@kumar303 kumar303 merged commit a7ca6ea into mozilla:master Mar 22, 2016
@kumar303 kumar303 deleted the build-filter branch March 22, 2016 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants