feat: deprecated logger in client & add Logger in ClientOption#738
Conversation
findleyr
left a comment
findleyr
left a comment
There was a problem hiding this comment.
LGTM following one minor nit.
I've already corrected it. PTAL. |
| if options != nil { | ||
| opts = *options | ||
| } | ||
| options = nil // prevent reuse |
There was a problem hiding this comment.
How does this prevent reuse?
As far as I can tell, it does so only for the current function, which is short.
There was a problem hiding this comment.
cause i see the func NewServer (which in mcp/server.go) do this, i thought this is the code style of this program.😂
If you think it's unnecessary, I will remove it.
| // | ||
| // If non-nil, the provided options configure the Client. | ||
| func NewClient(impl *Implementation, opts *ClientOptions) *Client { | ||
| func NewClient(impl *Implementation, options *ClientOptions) *Client { |
There was a problem hiding this comment.
Why rename to options? The caller knows the type, which already says Options?
There was a problem hiding this comment.
Similar above, I just keep the code style consistent.
If you think this will cause confusion, I will rename back.
3 participants
Fix #708