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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Mar 5, 2020 •
edited

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

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

ematipico
ematipico suggested changes Mar 6, 2020
View reviewed changes
Copy link
Contributor

@ematipico ematipico left a comment

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

packages/webpack-cli/lib/utils/cli-flags.js Outdated Show resolved Hide resolved
packages/webpack-cli/lib/groups/AdvancedGroup.js Show resolved Hide resolved
test/no-hot/no-hot.test.js Show resolved Hide resolved
@webpack-bot
Copy link

@snitin315 Thanks for your update.

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

@ematipico Please review the new changes.

ematipico
ematipico suggested changes Mar 9, 2020
View reviewed changes
Copy link
Contributor

@ematipico ematipico left a comment

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)

test/no-hot/no-hot.test.js Outdated Show resolved Hide resolved
@snitin315

This comment has been minimized.

Sign in to view
@snitin315 snitin315 requested a review from ematipico March 9, 2020 15:42
@snitin315 snitin315 mentioned this pull request Mar 17, 2020
Merged
@snitin315 snitin315 requested review from jamesgeorge007 and removed request for ematipico March 18, 2020 14:06
alexander-akait
alexander-akait requested changes Mar 18, 2020
View reviewed changes
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.

We should suspend this work until we merge #1347

@snitin315 snitin315 mentioned this pull request May 27, 2020
Merged
@snitin315 snitin315 deleted the feat/no-hot branch February 24, 2023 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ematipico ematipico ematipico requested changes

@alexander-akait alexander-akait alexander-akait requested changes

@jamesgeorge007 jamesgeorge007 Awaiting requested review from jamesgeorge007

Assignees
No one assigned
Labels
PR: CI-ok PR: reviewed-changes-requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@snitin315 @webpack-bot @ematipico @alexander-akait @jamesgeorge007