Use variadic function to pass parameters instead of Config struct#454
Use variadic function to pass parameters instead of Config struct#454peterbourgon merged 2 commits intogo-kit:masterfrom
Conversation
Using function to pass optional params makes the API more consistent with the rest of go-kit packages. For go-kit#427
peterbourgon
left a comment
peterbourgon
left a comment
There was a problem hiding this comment.
Yep, I'm basically into it. On review I think it's OK that the functional option operates on an unexported struct. Just a couple of tiny things to look at. Thanks so much for the effort!
| // will be squelched, and ErrNotAllowed returned. | ||
| Allowed []string | ||
| // New wraps the logger and implements level checking. See the commentary on the | ||
| // Option functions for a detailed description of how to configure levels. |
There was a problem hiding this comment.
Would you mind adding an extra sentence of commentary describing the default behavior with no options set?
| level.Allowed(level.AllowWarnAndAbove()), | ||
| level.ErrNotAllowed(myError), | ||
| } | ||
| logger := level.New(log.NewNopLogger(), opts...) |
There was a problem hiding this comment.
Any reason not to inline these? I'm not fussed, just curious.
There was a problem hiding this comment.
No specific reason. I usually prefer to create a slice when I have multiple options because it looks more organized, especially if there's a chance the list of options will grow in the future.
I'd be glad to change it if you have the opposite preference.
|
Brilliant, thanks for the contribution! |
2 participants
Using function to pass optional params makes the API more consistent
with the rest of go-kit packages.
For #427
Following up on discussion in #427 (comment)
I thought it was important to get this change in before #449 and ultimately 1.0.0 to avoid making breaking changes in a stable API.