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

refactor: apply webpack-defaults #54

Merged
merged 7 commits into from
Jun 24, 2017
Merged

Conversation

joshwiens
Copy link
Member

@joshwiens joshwiens commented Apr 8, 2017

Intended to be merged before & released with #26 & #45 as a part of 1.0.0 on a beta dist-tag once this has been finished and properly tested.

This is as close to inline with the defaults eslint config as this is going to get without making non-trivial structural changes to the lib which isn't something we should really be doing without a proper set of validations.


- refactor: Apply webpack-defaults 

BREAKING CHANGE: Enforces `peerDependencies` of `"webpack": ">= 3.0.0-rc.0 || ^3.0.0"`. 

BREAKING CHANGE: Enforces `engines` of `"node": ">=4.3.0 < 5.0.0 || >= 5.10`

BREAKING CHANGE: Remove loose dependency on Node Zopfli, which has been extracted to it's own plugin https://github.com/webpack-contrib/zopfli-webpack-plugin

Closes #52

@joshwiens joshwiens self-assigned this Apr 9, 2017
src/index.js Outdated
this.algorithm = options.algorithm || 'gzip';
this.filename = options.filename || false;
this.compressionOptions = {};
if (typeof this.algorithm === 'string') {
Copy link

Choose a reason for hiding this comment

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

I would use more newlines before ifs but maybe that's just me. 👍

src/index.js Outdated
} else {
const zlib = require('zlib');
this.algorithm = zlib[this.algorithm];
if (!this.algorithm) throw new Error('Algorithm not found in zlib');
Copy link

Choose a reason for hiding this comment

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

This is another case I would rather force by linting. if (...) {} always as it bites too easily.

Copy link
Member

Choose a reason for hiding this comment

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

Newlines between logical/lexical blocks is 👍 imho. Better more then none in general 😛

Copy link
Member Author

@joshwiens joshwiens May 1, 2017

Choose a reason for hiding this comment

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

@bebraw - last i checked we do enforce if (...) {} with the latest version of the eslint preset.

Copy link

Choose a reason for hiding this comment

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

@d3viant0ne Ok, great. 👍

@michael-ciniawsky michael-ciniawsky changed the title refactor: Apply webpack-defaults refactor: add webpack-defaults Apr 9, 2017
@michael-ciniawsky michael-ciniawsky changed the title refactor: add webpack-defaults refactor: apply webpack-defaults Apr 9, 2017
package.json Outdated
"main": "dist/cjs.js",
"files": [
"dist"
],
Copy link
Member

Choose a reason for hiding this comment

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

"repository": {
    "type": "git",
    "url": "git+https://github.com/webpack-contrib/compression-webpack-plugin.git"
  },
  "bugs": {
    "url": "https://github.com/webpack-contrib/compression-webpack-plugin/issues"
  },
  "homepage": "https://webpack.js.org",

@joshwiens joshwiens mentioned this pull request May 2, 2017
@joshwiens
Copy link
Member Author

Need to fix a few things in the pacakge.json / readme before this merges.

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented May 2, 2017

nodemon && serve:dev 🔪 since webpack-defaults >= 0.4.6, I can take the README aswell but I'm not figthing for it either 😛

@joshwiens
Copy link
Member Author

It's a local branch, if you want to throw your repo standards changes in here go nuts.

@michael-ciniawsky michael-ciniawsky added this to Refactor in Dashboard Jun 9, 2017
@joshwiens joshwiens merged commit f6f8c6c into master Jun 24, 2017
@joshwiens joshwiens deleted the d3viant0ne-WebpackDefault branch June 24, 2017 02:38
@michael-ciniawsky michael-ciniawsky removed this from Refactor in Dashboard Jun 27, 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.

webpack-defaults upgrade
3 participants