Skip to content

fix: show warning for non-web targets #3094

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Mar 19, 2021

  • 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?

Yes

Motivation / Use-Case

fixes #2104

Breaking Changes

None

Additional Info

None

@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #3094 (71a9eb9) into master (257e2eb) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3094      +/-   ##
==========================================
+ Coverage   95.69%   95.71%   +0.01%     
==========================================
  Files          34       34              
  Lines        1279     1284       +5     
  Branches      368      370       +2     
==========================================
+ Hits         1224     1229       +5     
  Misses         51       51              
  Partials        4        4              
Impacted Files Coverage Δ
lib/Server.js 96.03% <100.00%> (ø)
lib/utils/DevServerPlugin.js 98.91% <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 257e2eb...71a9eb9. Read the comment docs.

lib/Server.js Outdated
if (this.options.liveReload) {
this.logger.info(`Live reload will not work with a non-web target.`);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

It is warning, also better keep it inside DevServerPlugin, in future webpack-dev-server will be plugin, so we need put logic inside plugin

Copy link
Member Author

Choose a reason for hiding this comment

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

updated 👍🏻

@snitin315 snitin315 force-pushed the fix/non-web-target branch from 43fca8f to 790f08f Compare March 19, 2021 12:17
rishabh3112
rishabh3112 previously approved these changes Mar 28, 2021
@anshumanv
Copy link
Member

need rebase

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

Successfully merging this pull request may close these issues.

4 participants