Skip to content

chore: rename isLocalIp flag #2672

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

Merged
merged 5 commits into from
Jul 16, 2020
Merged

Conversation

rishabh3112
Copy link
Member

  • This is a bugfix
  • This is a feature
  • This is a code refactor
  • This is a test update
  • This is a docs update
  • This is a metadata update

For Bugs and Features; did you add new tests?

No (would add if needed)

Motivation / Use-Case

Keeping flags' syntax consistent overall

Breaking Changes

Maybe?

Additional Info

webpack/webpack-cli#1640

@knagaitsev
Copy link
Collaborator

Probably should be done for liveReload CLI flag as well. We can ignore serveIndex because it will be replaced in this PR: #2670.

Yes, this a breaking change, as it is --useLocalIp vs --use-local-ip.

@evilebottnawi Do you think this should go on v4 due to breaking changes?

@alexander-akait
Copy link
Member

@Loonride Yes, we should do for liveReload, I think it is more bug fixes, because webpack-cli also in rc, and do not use this

@alexander-akait
Copy link
Member

But let's merge it for v4

@alexander-akait alexander-akait changed the base branch from master to v4 July 13, 2020 16:48
@codecov
Copy link

codecov bot commented Jul 13, 2020

Codecov Report

Merging #2672 into v4 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##               v4    #2672   +/-   ##
=======================================
  Coverage   92.51%   92.51%           
=======================================
  Files          37       37           
  Lines        1310     1310           
  Branches      354      354           
=======================================
  Hits         1212     1212           
  Misses         93       93           
  Partials        5        5           

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 6b667b4...51bb2a0. Read the comment docs.

@knagaitsev
Copy link
Collaborator

We should also update in here then, correct? https://github.com/webpack/webpack-dev-server/blob/master/bin/options.js

@rishabh3112
Copy link
Member Author

@Loonride will update the PR with requested changes soon.
It's been long time since I dig into dev server's codebase, so wasn't well aware of implications of those 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.

/cc @hiroppy

@alexander-akait alexander-akait merged commit 4cb26b1 into webpack:v4 Jul 16, 2020
@alexander-akait
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants