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: code #2280

Merged
merged 2 commits into from Dec 31, 2020
Merged

refactor: code #2280

merged 2 commits into from Dec 31, 2020

Conversation

alexander-akait
Copy link
Member

What kind of change does this PR introduce?

refactoring

Did you add tests for your changes?

Yes

If relevant, did you update the documentation?

No

Summary

Refactor code + resolve TODO + tests

Does this PR introduce a breaking change?

No

Other information

More powerful option creator

@codecov
Copy link

codecov bot commented Dec 31, 2020

Codecov Report

Merging #2280 (c0c1eb2) into master (a3092ef) will increase coverage by 2.94%.
The diff coverage is 79.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2280      +/-   ##
==========================================
+ Coverage   65.11%   68.06%   +2.94%     
==========================================
  Files          63       63              
  Lines        2442     2464      +22     
  Branches      552      566      +14     
==========================================
+ Hits         1590     1677      +87     
+ Misses        852      787      -65     
Impacted Files Coverage Δ
packages/webpack-cli/lib/webpack-cli.js 90.01% <76.00%> (+7.82%) ⬆️
packages/webpack-cli/lib/utils/cli-flags.js 97.43% <92.30%> (+51.28%) ⬆️
packages/migrate/src/index.ts 88.88% <0.00%> (+4.93%) ⬆️
packages/utils/src/prop-types.ts 100.00% <0.00%> (+16.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3092ef...c0c1eb2. Read the comment docs.

@alexander-akait alexander-akait merged commit 2b5874e into master Dec 31, 2020
@alexander-akait alexander-akait deleted the refactor-code branch December 31, 2020 15:43
@delfikpro
Copy link

delfikpro commented Jan 2, 2021

Absolutely loved the new parameter logic of runCLI(...)

runCLI(['--config', webpackConfigPath, ...webpackArgs])

--config ... part is now ignored, causing not just an immediate crash, but rather weird and glitchy behavior. Took me two hours to investigate and fix.

Now webpack-cli wrappers that rely on runCLI need to be updated to pass two additional arguments:

runCLI(['', '', '--config', webpackConfigPath, ...webpackArgs])

@alexander-akait
Copy link
Member Author

@delfikpro If you need we can add third argument runCLI(['--config', webpackConfigPath, ...webpackArgs], { from: 'user' }) (and it will work), you need two empty '' because commader wait node ./path/to/script.js --arg value --arg value

@alexander-akait
Copy link
Member Author

alexander-akait commented Jan 3, 2021

Anyway you should not use bootstrap.js file, it is internal, please use CLI.js class (look at bootstrap.js source code), it is no documented, because it is not public right now, but I think it is safe to use

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.

None yet

3 participants