Skip to content

refactor: setupFeatures method #1877

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 2 commits into from
May 15, 2019
Merged

refactor: setupFeatures method #1877

merged 2 commits into from
May 15, 2019

Conversation

alexander-akait
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 need

Motivation / Use-Case

Refactor

Breaking Changes

No

Additional Info

No

@alexander-akait alexander-akait requested a review from hiroppy as a code owner May 15, 2019 13:03
);

this.options.setup(app, this);
this.setupSetupFeature();
Copy link
Member Author

Choose a reason for hiding this comment

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

Weird name, but we remove this in future so we can leave this as is

});
} else {
this._watch(contentBase);
}
Copy link
Member Author

@alexander-akait alexander-akait May 15, 2019

Choose a reason for hiding this comment

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

I see a bug here, when you use string we check on remote, but if you use array we don't check this :trollface: Let's fix it in other PR

Copy link
Member

Choose a reason for hiding this comment

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

Can you create an issue? (or already existed?)

Copy link
Member Author

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented May 15, 2019

Codecov Report

Merging #1877 into master will increase coverage by 0.38%.
The diff coverage is 97.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1877      +/-   ##
==========================================
+ Coverage   87.92%   88.31%   +0.38%     
==========================================
  Files          14       14              
  Lines         787      813      +26     
  Branches      259      258       -1     
==========================================
+ Hits          692      718      +26     
  Misses         82       82              
  Partials       13       13
Impacted Files Coverage Δ
lib/Server.js 86.27% <97.47%> (+0.82%) ⬆️

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 ab48526...602c914. Read the comment docs.

@alexander-akait
Copy link
Member Author

/cc @hiroppy

Copy link
Member

@hiroppy hiroppy left a comment

Choose a reason for hiding this comment

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

💯

@hiroppy hiroppy merged commit d1c319c into master May 15, 2019
@hiroppy hiroppy deleted the refactor-features branch May 15, 2019 15:50
EslamHiko pushed a commit to EslamHiko/webpack-dev-server that referenced this pull request May 15, 2019
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.

3 participants