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

Show warnings by default #15

Closed
lucaswerkmeister opened this issue Feb 19, 2022 · 1 comment
Closed

Show warnings by default #15

lucaswerkmeister opened this issue Feb 19, 2022 · 1 comment
Milestone

Comments

@lucaswerkmeister
Copy link
Owner

lucaswerkmeister commented Feb 19, 2022

Currently, the Node backend doesn’t show warnings by default, because I was worried about printing warnings in a CLI application; I had a vague idea to, some time after the 1.0 release, do a code search for m3api users, and consider showing warnings by default in a future 2.0 version if it looked like this wouldn’t cause much breakage.

After thinking this over for a bit longer, I think I’ve changed my mind: API warnings are important enough that they should be shown by default. Most users of the library won’t be CLI applications, and the benefit to those users of printing warnings by default should outweigh the risk of confusing users of CLI applications. Also, this will make example / model code for m3api shorter, since we won’t need to include warn: console.warn anymore, and it will simplify the internal code, since the default warn handler will no longer vary by backend.

@lucaswerkmeister lucaswerkmeister added this to the 1.0.0 release milestone Feb 19, 2022
@lucaswerkmeister
Copy link
Owner Author

(The Node backend does currently show warnings by default, but only if NODE_ENV is explicitly set to development, and I think it’s too much to hope that this will be common. We could try to flip this around – show warnings by default unless NODE_ENV is set to production – but I don’t think this is worth it. Many bots and tool backends will want to see warnings in “production” as well anyways.)

lucaswerkmeister added a commit that referenced this issue Jun 4, 2022
No longer needed since v0.6.0 (specifically commit b2af5ba, #15).
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

No branches or pull requests

1 participant