mcp,design: revert 'content' back to an interface type#29
Merged
Conversation
findleyr
force-pushed
the
findleyr/content
branch
3 times, most recently
from
June 25, 2025 14:25
90dec94 to
77f7e59
Compare
Contributor
|
Truncated commit message. |
Contributor
Author
Whoops (I copy-pasted to update the PR description). Fixed. Still getting used to GitHub. |
jba
requested changes
Jun 25, 2025
| 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"` |
Contributor
There was a problem hiding this comment.
We had talked about using the same interface-plus-concrete-types pattern here, for consistency. Why aren't we?
Contributor
Author
There was a problem hiding this comment.
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?
findleyr
force-pushed
the
findleyr/content
branch
from
June 25, 2025 16:12
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.
findleyr
force-pushed
the
findleyr/content
branch
from
June 25, 2025 17:07
1a78352 to
6543b59
Compare
jba
approved these changes
Jun 25, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.