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

Add disableBrowserLogging option #28 #37

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

musps
Copy link

@musps musps commented Oct 3, 2020

Hello,

From the issue #28, someone asked if it was possible to have an option to disable logs appearing in the browser console.

Option

option type default value
disableBrowserLogging boolean false

This option will disable the function print to be executed internally and will not print the logs in the browser console.

Usage

const $logger = Logger({
  url: '/test/api/log',
  disableBrowserLogging: true
});

@mstuart
Copy link
Contributor

mstuart commented Oct 5, 2020

@mnicpt @bluepnume can you take a look at ^^?

@mnicpt
Copy link
Contributor

mnicpt commented Oct 6, 2020

Yes. @musps I think disableBrowserLogging would be a better name since it's already enabled. Can you update this?

@mnicpt
Copy link
Contributor

mnicpt commented Oct 6, 2020

@musps Can you revert the /dist changes. This will happen automatically when we publish.

@musps musps changed the title Add enableBrowserLogging option #28 Add disableBrowserLogging option #28 Oct 6, 2020
@musps
Copy link
Author

musps commented Oct 6, 2020

@mnicpt Both are done!

Copy link
Contributor

@mnicpt mnicpt left a comment

Choose a reason for hiding this comment

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

Thanks, @musps, for taking this on! Looks great! 🎉

@zjye-idealhub
Copy link

any chance to release this enhancement soon?

@orens
Copy link

orens commented Dec 28, 2020

Thank you! looking forward for this PR to be merged as well!

One note, though, if it's not too much trouble I think it makes for a better API to have a browser log level instead of a flag. For our needs, for instance, it would be preferable to have only errors emitted to console and everything allowed by logLevel sent to BE. I guess this is not such a rare use case for other users as well.

You may even want to add a level above error (SUPPRESS? IGNORE?) which will cover the 'complete disable' use case, instead of another argument.

If required (provided you approve of this direction) I think I can make this change, building off @musps great work

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.

5 participants