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[utils]: opt to use config schema from core #1655

Merged
merged 7 commits into from
Jul 1, 2020

Conversation

anshumanv
Copy link
Member

What kind of change does this PR introduce?

cleanup

Did you add tests for your changes?
Not yet

If relevant, did you update the documentation?
NA

Summary

  • Config options are hardcoded which makes it tough to move forward with updates in webpack, use config keys from core directly

Does this PR introduce a breaking change?
No

Other information

@anshumanv anshumanv requested a review from a team as a code owner June 27, 2020 15:13
@anshumanv
Copy link
Member Author

Unusual test failing, taking a look

@anshumanv anshumanv changed the title chore[utils]: opt to use config schema from core feat[utils]: opt to use config schema from core Jun 28, 2020
@alexander-akait
Copy link
Member

@anshumanv yep, can you look why sometime tests failed?

@anshumanv
Copy link
Member Author

@evilebottnawi I checked and it seems the error is specific to this PR(different from random failures) -
image

Though I'm not sure why would everything break due to this change, runs fine locally.

@alexander-akait
Copy link
Member

@anshumanv yep, very strange

@anshumanv
Copy link
Member Author

@webpack/cli-team need small help, can you guys take a look at this? Unable to figure out.

@snitin315
Copy link
Member

@anshumanv ,

for webpack@4 config is undefined -

Screenshot at 2020-07-01 16-56-08

for webpack@5

Screenshot at 2020-07-01 16-59-17

@alexander-akait
Copy link
Member

Need add check isWebpack5

@anshumanv
Copy link
Member Author

Ah yes got it, thanks for looking @snitin315 @evilebottnawi 👍
Fix in near future

@anshumanv
Copy link
Member Author

@evilebottnawi @snitin315 review ready 👍

Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

Perfect 💯

@anshumanv anshumanv merged commit c2095ac into webpack:next Jul 1, 2020
@anshumanv anshumanv deleted the utils/cleanup branch July 1, 2020 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants