-
Notifications
You must be signed in to change notification settings - Fork 783
refactor: replace console.warn with process.emitWarning#954
refactor: replace console.warn with process.emitWarning#954RyanZim merged 1 commit intojprichardson:masterfrom lamweili:refactor/warning-mechanism
Conversation
To have consistent warnings from #953 (comment) by @JPeer264
Yes
process.emitWarningsounds also like a great option to add (just my opinion on it). However, a separate PR would be nice if you want to go the extra mile as we squash our PRs. @manidlou @RyanZim @jprichardson any objections?
Prints out via process.emitWarning in the following format:
|
Should we consider this: https://nodejs.org/api/process.html#avoiding-duplicate-warnings? |
|
My motivation was to retain the behavior of the Should we change it to use https://nodejs.org/api/process.html#avoiding-duplicate-warnings instead, so it only warns on the first time? What do you guy think? Let me know and I'll make the adjustments. |
JPeer264
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with either. Thanks for the changes and your efforts :)