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.

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

feat: add webpack as argument to before and after options #1760

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?

Yes

Motivation / Use-Case

Allow to get information from webpack in before and after

Breaking Changes

No

Additional Info

No

@alexander-akait alexander-akait requested a review from hiroppy as a code owner April 4, 2019 14:20
@alexander-akait
Copy link
Member Author

/cc @hiroppy We need setup curly in eslint to avoid mixed style with and without brackets

@hiroppy
Copy link
Member

hiroppy commented Apr 4, 2019

Which do you prefer modifying webpack-dev-server's eslint config or eslint-config-webpack?

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.

lgtm if ci is green

@alexander-akait
Copy link
Member Author

@codecov
Copy link

codecov bot commented Apr 4, 2019

Codecov Report

Merging #1760 into master will increase coverage by 0.69%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1760      +/-   ##
==========================================
+ Coverage   86.85%   87.54%   +0.69%     
==========================================
  Files           9        9              
  Lines         578      578              
  Branches      170      170              
==========================================
+ Hits          502      506       +4     
+ Misses         63       60       -3     
+ Partials       13       12       -1
Impacted Files Coverage Δ
lib/Server.js 83.16% <100%> (+1.03%) ⬆️

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 31dfe22...cf2aeae. Read the comment docs.

@alexander-akait alexander-akait merged commit 0984d4b into master Apr 4, 2019
@alexander-akait alexander-akait deleted the feat-add-compiler-argument-to-before-and-after-option branch April 4, 2019 14:50
@hiroppy
Copy link
Member

hiroppy commented Apr 4, 2019

@evilebottnawi eslint-config-prettier overwrite this rule.

I found that eslint-config-prettier was overriding the rules.(curly: off)

So, we have to change the order https://github.com/webpack/webpack-dev-server/blob/master/.eslintrc#L2 or write this rule to webpack-dev-server's eslintrc.

@alexander-akait
Copy link
Member Author

hm, it is strange, need investigate why prettier disable curly, because prettier don't add { and }

@hiroppy
Copy link
Member

hiroppy commented Apr 4, 2019

@alexander-akait
Copy link
Member Author

@hiroppy
Copy link
Member

hiroppy commented Apr 4, 2019

Yeah, but we are setting "extends": ["webpack", "prettier"] now, so the setting of eslint-config-webpack is overwritten because the setting of curly is written in eslint-config-prettier.

@alexander-akait
Copy link
Member Author

@hiroppy we need fix 👍

@hiroppy
Copy link
Member

hiroppy commented Apr 4, 2019

@evilebottnawi for now, I created #1761.

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.

2 participants