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

fix: defer setting default mode to core #2095

Merged
merged 4 commits into from Nov 11, 2020

Conversation

anshumanv
Copy link
Member

What kind of change does this PR introduce?

fix

Did you add tests for your changes?
Yes

If relevant, did you update the documentation?
Not needed I think

Summary

  • Defer setting default mode to core so it can print warnings and stuff

Does this PR introduce a breaking change?
Nope

Other information

snitin315
snitin315 previously approved these changes Nov 10, 2020
rishabh3112
rishabh3112 previously approved these changes Nov 10, 2020
@codecov
Copy link

codecov bot commented Nov 10, 2020

Codecov Report

Merging #2095 (560ebb2) into master (2d6e5c6) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2095      +/-   ##
==========================================
+ Coverage   68.61%   68.65%   +0.03%     
==========================================
  Files          74       74              
  Lines        2390     2393       +3     
  Branches      508      510       +2     
==========================================
+ Hits         1640     1643       +3     
  Misses        750      750              
Impacted Files Coverage Δ
packages/webpack-cli/lib/webpack-cli.js 93.35% <100.00%> (+0.06%) ⬆️

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 2d6e5c6...560ebb2. Read the comment docs.

snitin315
snitin315 previously approved these changes Nov 10, 2020
@alexander-akait
Copy link
Member

/cc @anshumanv need fix tests

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Feel free to merge on green

Comment on lines 29 to 30
'--mode',
'production',
Copy link
Member

Choose a reason for hiding this comment

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

I think it will be better to define mode: production inside the config.

Copy link
Member

Choose a reason for hiding this comment

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

Yep, let's do it

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, let's do it

@alexander-akait
Copy link
Member

@anshumanv We need fix test for stats and watch, let's add mode: development

@anshumanv
Copy link
Member Author

should be good now

@alexander-akait alexander-akait merged commit 3eb410e into webpack:master Nov 11, 2020
@alexander-akait
Copy link
Member

Thanks

@anshumanv anshumanv deleted the def-mode branch November 11, 2020 17:17
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

5 participants