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

feat(logger): Allow logger to be set in config #340

Merged
merged 2 commits into from
Apr 19, 2018

Conversation

bengourley
Copy link
Contributor

Previously configuration would happen before the client.logger() method could be called, so the "[bugsnag] loaded!" message would be printed before the logging could be disabled. This commit allows the logger to be set in the configuration object. Logging can be completely disabled by passing logger: null.

Fixes #302

Previously configuration would happen _before_ the client.logger() method could be called, so the
"[bugsnag] loaded!" message would be printed before the logging could be disabled. This commit
allows the logger to be set in the configuration object. Logging can be completely disabled by
passing `logger: null`.

Fixes #302
The previous commit adding logger to the configuration increased the bundle size. This should
counterbalance that added weight.
@bengourley
Copy link
Contributor Author

The reason the error messages can have the property name removed is because we build a message containing the key already in configure():

err.errors = map(validity.errors, (err) => `${err.key} ${err.message} \n ${err.value}`)

@bengourley bengourley requested a review from a team April 19, 2018 14:54
@@ -129,18 +123,6 @@ module.exports = (opts, userPlugins = []) => {
: bugsnag
}

const getPrefixedConsole = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is only available/used inside the browser/index.js exported function correct?
So it's removal shouldn't be a breaking change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, it was "private" to that module. It's been moved to browser/config.js where it is now used.

Copy link
Contributor

@Cawllec Cawllec left a comment

Choose a reason for hiding this comment

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

LGTM

@bengourley bengourley merged commit e71db02 into master Apr 19, 2018
@bengourley bengourley deleted the bengourley/logger-config branch May 3, 2018 10:58
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.

None yet

2 participants