Skip to content
This repository was archived by the owner on Jul 21, 2021. It is now read-only.

add a Logger interface and setter for the logging Printf calls #61

Merged
merged 2 commits into from
Jun 5, 2015

Conversation

theckman
Copy link
Contributor

@theckman theckman commented Jun 5, 2015

By adding this Logger interface, and by fulfilling the interface with a function that has no body, you can silence the log messages from the *zk.Conn instance. The Connect() and ConnectWithDialer() functions both set the logger up by default to log to Stderr.

Fixes #60.

By adding this Logger interface, and by fulfilling the interface with a function that has no body, you can silence the log messages from the *zk.Conn instance. The `Connect()` and `ConnectWithDialer()` functions both set the logger up by default to log to `Stderr`.

Fixes samuel#60.
@samuel
Copy link
Owner

samuel commented Jun 5, 2015

Thanks for putting together this patch.

@@ -143,6 +150,9 @@ func ConnectWithDialer(servers []string, sessionTimeout time.Duration, dialer Di
if dialer == nil {
dialer = net.DialTimeout
}

newLogger := log.New(os.Stderr, "", log.LstdFlags)
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be best to use the default logger so that if someone has already used log.SetOutput then it'll work as expected (as before).

type defaultLogger struct{}

func (defaultLogger) Printf(format string, v ...interface{}) {
    log.Printf(format, v...)
}
newLogger := defaultLogger{}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely. I should have gone with that pattern in the first place. I should go further and export the type. That way, someone could fall back to the original logger, if needed, without needing to dive in to the source to see how we create it.

* remove custom formatting in one of the `Printf()` statements to add a newline character at the end
* changed how the default logger is instantiated -- new defaultLogger type, and an exported variable that is an instance of it
@theckman theckman force-pushed the add_logging_interface branch from ecb8e8a to 24aa123 Compare June 5, 2015 19:53
@theckman
Copy link
Contributor Author

theckman commented Jun 5, 2015

Okay, I think I properly addressed your feedback. Please let me know if there's anything else that needs done.

@samuel
Copy link
Owner

samuel commented Jun 5, 2015

Looks great. Thanks!

samuel added a commit that referenced this pull request Jun 5, 2015
add a Logger interface and setter for the logging Printf calls
@samuel samuel merged commit f575d97 into samuel:master Jun 5, 2015
@theckman theckman deleted the add_logging_interface branch June 5, 2015 23:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants