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.

Sign up for GitHub

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

Jump to bottom

fix: multiple config compilation error (rebase) #1468

Closed
wants to merge 10 commits into from
Closed

fix: multiple config compilation error (rebase) #1468

wants to merge 10 commits into from

Conversation

aman29271
Copy link
Contributor

What kind of change does this PR introduce?
A bugfix which closes #887

Did you add tests for your changes?
Yes
If relevant, did you update the documentation?
Nope

Does this PR introduce a breaking change?
No

Other information

@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@alexander-akait
Copy link
Member

/cc @webpack/cli-team Should we accept bug fixes for v3 or we should focus on v4?

@chefsrini
Copy link

Do we have an ETA for the merge

alexander-akait
alexander-akait approved these changes May 12, 2020
View reviewed 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 @webpack/cli-team

@snitin315
Copy link
Member

Also, can you please add a test case for --info-verbosity verbose with multiple configs as per the Issue, So that we can ensure we don't have the same problem anymore?

@webpack-bot
Copy link

@aman29271 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@evilebottnawi Please review the new changes.

@aman29271
Copy link
Contributor Author

@snitin315 Thanks for pointing out. Tests has been added.

@aman29271 aman29271 requested a review from alexander-akait May 13, 2020 09:54
alexander-akait
alexander-akait reviewed May 13, 2020
View reviewed changes
test/binCases/entry/multiple-config/multiple-config.test.js Outdated
expect(code).toBe(0);
expect(code).toBeDefined();
expect(code).not.toBeNull();
expect(summary).toEqual(expect.anything());
Copy link
Member

Choose a reason for hiding this comment

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

Bad idea to do it, no need test on anything

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@evilebottnawi should I remove it? 🤔

@aman29271 aman29271 requested a review from alexander-akait May 27, 2020 18:42
@alexander-akait
Copy link
Member

Fixed in master, anyway big thanks for PR

This was referenced Sep 15, 2021
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snitin315 snitin315 snitin315 left review comments

@alexander-akait alexander-akait Awaiting requested review from alexander-akait

Assignees
No one assigned
Labels
PR: conflict PR: review-outdated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG]: Multiple config throws error calling tap on undefined in webpack-cli
5 participants
@aman29271 @webpack-bot @alexander-akait @chefsrini @snitin315