Skip to content

Make log & warn functions replaceable #124

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 4 commits into from
Sep 19, 2016
Merged

Make log & warn functions replaceable #124

merged 4 commits into from
Sep 19, 2016

Conversation

alanpearce
Copy link
Contributor

By allowing overrides of the functions used for logging, we can allow the output from this middleware to be processed according to the logging library used in the application.

@SpaceK33z
Copy link
Member

There is also a reporter option, that does about the same. There is only one console.log that is not inside this reporter. Could you maybe move that console.log over there instead?

@alanpearce
Copy link
Contributor Author

Well, the reason I didn't touch reporter is because it seems non-trivial to replace as a library user, and then relies on the API not breaking over time. Also, from the name, I assumed it would be a function run at the end of a compilation to format the output. IMO it should not even be used to log that the bundle is invalid, just to report the stats (or not) from a compilation.

I could do it, but it seems incorrect to me. I'd have to add some other state value or some other decision flag for the other console.log, and then what happens if maybe more logging is added later? More state values? Would it really be desirable to maintain a most-likely increasing number of switch cases rather than adding a log line, which could then be logged in a way that fits into whatever framework or log library the user may be using?

If you really think it should be done like this, please let me know what you'd like to have as the state value for the last log call outside of the reporter, or how to otherwise decide if this is a "waiting" event.

@SpaceK33z
Copy link
Member

Okay, fair enough. We can let the reporter method only be for processing the output.

Could you add a test for this and fix the broken tests? The relevant tests are in test/Reporter.test.js.

Alan Pearce added 3 commits September 19, 2016 10:49
 - console.info is never called any more
 - since both log/info calls are merged, increase the expected count
 of calls by one
Copy link
Member

@SpaceK33z SpaceK33z left a comment

Choose a reason for hiding this comment

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

Looks good.

@SpaceK33z SpaceK33z merged commit ab17c1b into webpack:master Sep 19, 2016
@SpaceK33z
Copy link
Member

Thanks!

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