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

lack of console.logger.warn() implementation #29

Open
wolfmic opened this issue Aug 18, 2024 · 5 comments
Open

lack of console.logger.warn() implementation #29

wolfmic opened this issue Aug 18, 2024 · 5 comments

Comments

@wolfmic
Copy link

wolfmic commented Aug 18, 2024

While working on this issue: ubiquity-os-marketplace/command-wallet#4
It appears that console.logger.warn() is not implemented yet on ubiquibot-logger
Is it useful to add it to this project?

@0x4007
Copy link
Member

0x4007 commented Aug 18, 2024

@gentlementlegen @Keyrxng rfc

@gentlementlegen
Copy link
Member

I think it would be nice to have a warn level. The problem is that currently the error actually displays yellow messages that looks like warning, see this code: https://github.com/gentlementlegen/ubiquibot-logger/blob/aca537b8457ef6a9a7e9064dc97b1929bf3f6e7d/src/pretty-logs.ts#L127

@Keyrxng
Copy link
Contributor

Keyrxng commented Aug 18, 2024

No I don't think so. The below comment is what you see when you hover over a traditional console.warn

The console.warn() function is an alias for error

The difference is that it renders in yellow (or close to) to represent a warning but our logger renders .error() in yellow and .fatal renders in red so it wouldn't add any value in my opinion.

Unless we removed .fatal as it's recommended to avoid using it anyway and then rendered .error() in red and .warn() in yellow?

Overall I'd say it adds little value to the current logger unless we remove fatal and rearrange the colour scheme

@Keyrxng
Copy link
Contributor

Keyrxng commented Aug 18, 2024

@gentlementlegen this made me realize that we don't offer the option to configure the logger from a partner standpoint it's whatever the author sets up context with. Should we be allowing this to be configured by the partner or should we use the most verbose as it will catch everything?

@gentlementlegen
Copy link
Member

@Keyrxng That might be handy although I think that the logger should usually be verbose for debugging. I guess we could achieve this through the configuration, but it would be the plugin author duty to expose this parameter inside the plugins, I don't think we can have the option globally.

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

4 participants