-
Notifications
You must be signed in to change notification settings - Fork 80
Use valid syntax for URIs in examples and tests #94
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
Use valid syntax for URIs in examples and tests #94
Conversation
examples/http_server.rb
Outdated
| resources: [ | ||
| MCP::Resource.new( | ||
| uri: "test_resource", | ||
| uri: "example://test_resource", |
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 might be preferable to use a more generic protocol name (e.g., https://). As for the example domain of the resource, something like .invalid (returns NXDOMAIN) could be considered.
For example, a value such as https://test_resource.invalid could be used.
test/mcp/server_test.rb
Outdated
|
|
||
| @resource = Resource.new( | ||
| uri: "test_resource", | ||
| uri: "test://test_resource", |
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.
Ditto.
|
Can you also update the usage of |
e31a881 to
f4697ac
Compare
|
This update looks good. Can you squash your commits into one? |
090380b to
3181af8
Compare
3181af8 to
8b92ce0
Compare
atesgoral
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.
TIL about the .invalid TLD. I've always been using example.com.
The spec appears to use only properly structured URIs, so the examples and tests should reflect that.