-
Notifications
You must be signed in to change notification settings - Fork 266
mcp,design: revert 'content' back to an interface type #29
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
Conversation
90dec94 to
77f7e59
Compare
|
Truncated commit message. |
Whoops (I copy-pasted to update the PR description). Fixed. Still getting used to GitHub. |
| Blob []byte `json:"blob,omitempty"` // if nil, then text; else blob | ||
| URI string `json:"uri,"` | ||
| MIMEType string `json:"mimeType,omitempty"` | ||
| Text string `json:"text,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had talked about using the same interface-plus-concrete-types pattern here, for consistency. Why aren't we?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did that, and it added a lot of code and complexity for little value. Unlike contents, this isn't a distinguished union: the presence of text or blob determines its type, and otherwise the types are the same. We can make it an interface, but maybe in a later CL?
77f7e59 to
1a78352
Compare
After some experience with the flattened version of Content, we see that it can easily lead to incorrect usage and is a harder API to read. Therefore, this CL changes back to a design similar to what we had prior to https://go.dev/cl/672415, though opting to promote content unmarshalling to the protocol types that use it, which leads to an overall simpler api.
1a78352 to
6543b59
Compare
After some experience with the flattened version of Content, we see that
it can easily lead to incorrect usage and is a harder API to read.
Therefore, this CL changes back to a design similar to what we had prior
to https://go.dev/cl/672415, though opting to promote content
unmarshalling to the protocol types that use it, which leads to an
overall simpler api.