Use valid syntax for URIs in examples and tests#94
Merged
atesgoral merged 1 commit intomodelcontextprotocol:mainfrom Jul 27, 2025
Merged
Conversation
koic
reviewed
Jul 27, 2025
examples/http_server.rb
Outdated
| resources: [ | ||
| MCP::Resource.new( | ||
| uri: "test_resource", | ||
| uri: "example://test_resource", |
Member
There was a problem hiding this comment.
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.
koic
reviewed
Jul 27, 2025
test/mcp/server_test.rb
Outdated
|
|
||
| @resource = Resource.new( | ||
| uri: "test_resource", | ||
| uri: "test://test_resource", |
Member
|
Can you also update the usage of |
e31a881 to
f4697ac
Compare
Member
|
This update looks good. Can you squash your commits into one? |
090380b to
3181af8
Compare
3181af8 to
8b92ce0
Compare
atesgoral
approved these changes
Jul 27, 2025
Contributor
atesgoral
left a comment
There was a problem hiding this comment.
TIL about the .invalid TLD. I've always been using example.com.
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
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.
The spec appears to use only properly structured URIs, so the examples and tests should reflect that.