Skip to content

feat(webpack-cli): add no-hot flag#1296

Closed
snitin315 wants to merge 1 commit into
webpack:nextfrom
snitin315:feat/no-hot
Closed

feat(webpack-cli): add no-hot flag#1296
snitin315 wants to merge 1 commit into
webpack:nextfrom
snitin315:feat/no-hot

Conversation

@snitin315

@snitin315 snitin315 commented Mar 5, 2020

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Added --no-hot flag to CLI.

Did you add tests for your changes?

YES

If relevant, did you update the documentation?

YES

Summary

Refers #1240

Refers #1012

Does this PR introduce a breaking change?

NO

Other information

@snitin315
snitin315 requested a review from a team as a code owner March 5, 2020 14:40
@webpack-bot

Copy link
Copy Markdown

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@ematipico ematipico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good work I think. We should cover more cases with tests

Comment thread packages/webpack-cli/lib/utils/cli-flags.js Outdated
Comment thread packages/webpack-cli/lib/groups/AdvancedGroup.js
Comment thread test/no-hot/no-hot.test.js
@webpack-bot

Copy link
Copy Markdown

@snitin315 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@ematipico Please review the new changes.

@ematipico ematipico left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please create the following tests:

  • with a configuration file where there's devServer: { hot: true }
  • a test for the serve command (webpack-dev-server consumes the flag)

Comment thread test/no-hot/no-hot.test.js Outdated
@snitin315

This comment has been minimized.

Comment thread packages/webpack-cli/__tests__/AdvancedGroup.test.js Outdated

@alexander-akait alexander-akait left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should suspend this work until we merge #1347

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.

5 participants