Forbid console.log and console.info
#835
Conversation
|
|
mfisher87
changed the title
console.log and console.info
mfisher87
force-pushed
the
forbid-console-log
branch
from
1b70c8e to
af6807f
Compare
|
Integration tests report: appsharing.space |
There was a problem hiding this comment.
Hi @mfisher87 Sorry that that fell through the cracks. Can we push it across the line? There are some conflicts to be resolved
Console calls, IMO, should be for debugging only. They're useful for users facing bugs to share information about any unexpected events. Instead of using `console.log` as a catch-all, I'd like to think more carefully about the purpose of each message, and either omit or log it as an error, warning, or debug message.
Remove log and info calls or convert them to other calls. Clean up some messages.
mfisher87
force-pushed
the
forbid-console-log
branch
from
af6807f to
6c5c0f8
Compare
|
I think it's ready to go now! |
Thanks @mfisher87, do you think it'd be wiser to add a short pointer in contributing guide as well? Otherwise I'll be happy to merge it as is. |
|
I think it's OK to just have this only expressed in the linter rules since it's easy to fix. I think the things that should go in the contributor docs are standards or conventions that are more nuanced, need some supplementary explanation, and/or can't be checked/fixed by a linter. I don't want the contributor docs to be hard to maintain! Though I'd be happy to add something to the contributor docs if you feel it would be valuable! |
|
Ahh I think it's ok then 👍, let's go ahead with this |
2 participants
Description
Console calls, IMO, should be for debugging only. They're useful for users facing bugs to share information about any unexpected events. Instead of using
console.logas a catch-all, I'd like to think more carefully about the purpose of each message, and either omit or log it as an error, warning, or debug message.What do you all think?
If I've removed any calls that you all feel are important, let's change them to
console.debugcalls?Checklist
Resolves #XXX.Failing lint checks can be resolved with:
pre-commit run --all-filesjlpm run lint📚 Documentation preview: https://jupytergis--835.org.readthedocs.build/en/835/
💡 JupyterLite preview: https://jupytergis--835.org.readthedocs.build/en/835/lite