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

refactor: replace console.warn with process.emitWarning #954

Merged
merged 1 commit into from Apr 16, 2022
Merged

refactor: replace console.warn with process.emitWarning #954

merged 1 commit into from Apr 16, 2022

Conversation

lamweili
Copy link
Contributor

@lamweili lamweili commented Apr 8, 2022

To have consistent warnings from #953 (comment) by @JPeer264

Yes process.emitWarning sounds 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:

image

@lamweili lamweili marked this pull request as draft Apr 8, 2022
@lamweili lamweili marked this pull request as ready for review Apr 8, 2022
@lamweili lamweili marked this pull request as draft Apr 8, 2022
@lamweili lamweili marked this pull request as ready for review Apr 8, 2022
Signed-off-by: Lam Wei Li <peteriman@mail.com>
@RyanZim
Copy link
Collaborator

@RyanZim RyanZim commented Apr 8, 2022

Should we consider this: https://nodejs.org/api/process.html#avoiding-duplicate-warnings?

@lamweili
Copy link
Contributor Author

@lamweili lamweili commented Apr 8, 2022

My motivation was to retain the behavior of the console.warn which warns duplicatedly if called multiple times.

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. 😊
@RyanZim @JPeer264 @manidlou @jprichardson

Copy link
Collaborator

@JPeer264 JPeer264 left a comment

I am fine with either. Thanks for the changes and your efforts :)

@RyanZim RyanZim merged commit baa9934 into jprichardson:master Apr 16, 2022
18 checks passed
@lamweili lamweili deleted the refactor/warning-mechanism branch Apr 17, 2022
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.

None yet

3 participants