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

chore(logger): remove logger package, inline webpack logger #1358

Merged
merged 9 commits into from
Mar 25, 2020

Conversation

anshumanv
Copy link
Member

What kind of change does this PR introduce?
Remove logger package.

Did you add tests for your changes?
Yes

If relevant, did you update the documentation?
Yes

Summary

  • Remove logger package, inline webpack logger

Does this PR introduce a breaking change?
No

Other information
NA

snitin315
snitin315 previously approved these changes Mar 21, 2020
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.

CI under maintenance, LGTM

himanshu010
himanshu010 previously approved these changes Mar 21, 2020
shivaylamba
shivaylamba previously approved these changes Mar 21, 2020
jamesgeorge007
jamesgeorge007 previously approved these changes Mar 22, 2020
@webpack-bot
Copy link

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

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 need more reviews

@anshumanv
Copy link
Member Author

@evilebottnawi rebased which removed your review, PTAL again. ✨

snitin315
snitin315 previously approved these changes Mar 23, 2020
@alexander-akait
Copy link
Member

/cc @webpack/cli-team need more reviews

rishabh3112
rishabh3112 previously approved these changes Mar 24, 2020
@anshumanv
Copy link
Member Author

PTAL @evilebottnawi @rishabh3112, let's get this in. 🌟

@anshumanv
Copy link
Member Author

Uhoh, taking look at tests.

@webpack-bot
Copy link

@anshumanv Thanks for your update.

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

@evilebottnawi Please review the new changes.

@@ -1,3 +1,3 @@
const { logger } = require('@webpack-cli/logger');
const { getLogger } = require('webpack/lib/logging/runtime');
Copy link
Member

Choose a reason for hiding this comment

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

webpack/lib/logging/runtime should not be used here. Use compiler.getInfrastructureLogger("webpack-cli") instead.

Copy link
Member Author

@anshumanv anshumanv Apr 6, 2020

Choose a reason for hiding this comment

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

Will update 👍

Copy link
Member

Choose a reason for hiding this comment

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

@sokra we use logging before run webpack and after, and when we do other actions which don't need webpack (like init/info/migrate/generate), so we can't use infrastructure logger

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.

None yet