-
Notifications
You must be signed in to change notification settings - Fork 177
Add backwards compatibility constructors and tests #395
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
- Updated integration tests to include assertions for `meta` being `null` in the `listTools`, `listResources`, `readResource`, and `callTool` results. - Renamed test cases in `SseServerIntegrationTest` for clarity.
…transport null check.
…ity for paginated request parameters, and enhance `meta` handling across SDK components.
devcrocod
left a comment
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 only have some comments about the code style
| * The argument's information for which completion options are requested. | ||
| */ | ||
| public val argument: CompleteRequestParams.Argument | ||
| public val argument: Argument |
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.
It’s better to refer to the class directly rather than importing it separately
this makes the code clearer and easier to understand:
CompleteRequestParams.Argument
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.
Idea complains about it but yes
| * Additional, context for generating completions. | ||
| */ | ||
| public val context: CompleteRequestParams.Context? | ||
| public val context: Context? |
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.
The same as with Argument
metabeingnullin thelistTools,listResources,readResource, andcallToolresults.SseServerIntegrationTestfor clarity.throw Error(...)witherror(...)for cleaner error handling.Motivation and Context
To verify backwards-compatibility after schema change
How Has This Been Tested?
CI, integration tests, regression tests (samples)
Breaking Changes
Types of changes
Checklist
Additional context