Skip to content
This repository has been archived by the owner on Dec 5, 2019. It is now read-only.

Refactor stuff #333

Merged
merged 5 commits into from Jul 30, 2018
Merged

Refactor stuff #333

merged 5 commits into from Jul 30, 2018

Conversation

alexander-akait
Copy link
Member

Briefly:

  1. Move some utils to class (we don't need extract require for some stuff).
  2. Merge some identical tests.
  3. Add test for validation.
  4. Rename all test (example sourceMap.test.js -> sourceMap-option.test.js)

@@ -56,8 +54,18 @@ class UglifyJsPlugin {
};
}

static isSourceMap(input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer the function as a helper. If you expose it it becomes a public API

Copy link
Member Author

Choose a reason for hiding this comment

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

@ooflorent yep, why:

  1. AbstractMinimizerWebpackPlugin can be based on this code and this helper should be public (I'm preparing to release it next week).
  2. We use source map only when we have errors/warnings and all logic for building errors/warnings already inside class (it is also require for AbstractMinimizerWebpackPlugin).

@alexander-akait
Copy link
Member Author

Merge as is, feel free to feedback

@alexander-akait alexander-akait merged commit 726874b into master Jul 30, 2018
@alexander-akait alexander-akait deleted the refactor-stuff branch July 30, 2018 10:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants