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

feat: add option to change the filename #51

Merged
merged 2 commits into from
Apr 8, 2017
Merged

feat: add option to change the filename #51

merged 2 commits into from
Apr 8, 2017

Conversation

ronen-e
Copy link
Contributor

@ronen-e ronen-e commented Apr 5, 2017

Optionally change file name

@jsf-clabot
Copy link

jsf-clabot commented Apr 5, 2017

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

@ronen-e Also Docs && Tests please :)

index.js Outdated
@@ -11,6 +11,7 @@ function CompressionPlugin(options) {
options = options || {};
this.asset = options.asset || "[path].gz[query]";
this.algorithm = options.algorithm || "gzip";
this.changeNewFileName = options.changeNewFileName || false;
Copy link
Member

Choose a reason for hiding this comment

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

this.changeNewFileName => this.name or something that is similiar terse 😛

Copy link

Choose a reason for hiding this comment

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

See how we did something similar in extract-text-webpack-plugin. Maybe a good solution to mimic.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@michael-ciniawsky I didn't see any tests for the existing code.
I would like to add to them if they exist.
Do you have any tests that you already run on the code?

Copy link

Choose a reason for hiding this comment

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

Ah, yeah... We would have to update this project to webpack-defaults to get testing infrastructure in place.

Copy link
Member

@michael-ciniawsky michael-ciniawsky Apr 6, 2017

Choose a reason for hiding this comment

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

@ronen-e Ooops.. 😛 forget what I said then 🙃 Just change tofilename + docs and good to go imho

@michael-ciniawsky michael-ciniawsky self-assigned this Apr 5, 2017
@michael-ciniawsky michael-ciniawsky changed the title Add option to change new file name feat: add option to change the filename Apr 6, 2017
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

@ronen-e Thx

@michael-ciniawsky michael-ciniawsky merged commit 9555370 into webpack-contrib:master Apr 8, 2017
joshwiens pushed a commit that referenced this pull request Apr 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants