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

Use process warnings for deprecations #1276

Merged
merged 22 commits into from
Jan 5, 2024

Conversation

eritbh
Copy link
Contributor

@eritbh eritbh commented Sep 7, 2021

Addresses #1274. Adds process warning emits alongside existing deprecation-related client warn emits. To avoid emitting the same process warning multiple times, uses a wrapper around process.emitWarning that checks if the given warning code has already been used, and if so, skips emitting the warning again.

lib/util/emitDeprecation.js Outdated Show resolved Hide resolved
lib/Client.js Outdated Show resolved Hide resolved
Copy link
Contributor Author

@eritbh eritbh left a comment

Choose a reason for hiding this comment

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

Changed structure around a bit in response to discussion activity. Looking for feedback on the codes themselves.

I also think it would be a good idea to rewrite some of these existing messages to include more context and provide migration paths away from the problem, rather than focusing on being short. How do we feel about this?

lib/util/emitDeprecation.js Outdated Show resolved Hide resolved
@eritbh
Copy link
Contributor Author

eritbh commented Sep 20, 2021

Rewrote the rest of the warning descriptions to be consistent with each other. I've elected not to copy these changes to warnings emitted on the client to avoid potentially breaking things, but if it's desired anyway, I can copy my changes to those as well. Please double check my grammar and format.

@eritbh eritbh changed the title Create deprecation helper, use for existing warns Use process warnings for deprecations Sep 20, 2021
@abalabahaha abalabahaha added this to the 0.17.x milestone Jun 29, 2022
Jam-Manbo pushed a commit to Manbo-js/manbo that referenced this pull request Sep 28, 2022
@eritbh
Copy link
Contributor Author

eritbh commented Jan 5, 2024

Merged latest dev in again and this branch still appears to work fine for me, though I'd guess there are additional deprecated behaviors that have been introduced in the meantime which will need to be handled in a successive PR.

@bsian03 bsian03 merged commit ae5f0f0 into abalabahaha:dev Jan 5, 2024
@eritbh eritbh deleted the deprecation-process-warnings branch January 5, 2024 18:58
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

5 participants