refactor: apply webpack-defaults #54
Conversation
src/index.js
Outdated
| this.algorithm = options.algorithm || 'gzip'; | ||
| this.filename = options.filename || false; | ||
| this.compressionOptions = {}; | ||
| if (typeof this.algorithm === 'string') { |
There was a problem hiding this comment.
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'); |
There was a problem hiding this comment.
This is another case I would rather force by linting. if (...) {} always as it bites too easily.
There was a problem hiding this comment.
Newlines between logical/lexical blocks is 👍 imho. Better more then none in general 😛
There was a problem hiding this comment.
@bebraw - last i checked we do enforce if (...) {} with the latest version of the eslint preset.
michael-ciniawsky
changed the title
michael-ciniawsky
changed the title
package.json
Outdated
| "main": "dist/cjs.js", | ||
| "files": [ | ||
| "dist" | ||
| ], |
There was a problem hiding this comment.
"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
added
pr_action: Review
semver: Major
flag: Needs Tests
and removed
breaking change
labels
|
Need to fix a few things in the pacakge.json / readme before this merges. |
|
|
|
It's a local branch, if you want to throw your repo standards changes in here go nuts. |
Intended to be merged before & released with #26 & #45 as a part of
1.0.0on a betadist-tagonce 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.
Closes #52