Skip to content

Conversation

@andyw8
Copy link
Contributor

@andyw8 andyw8 commented Jul 26, 2025

The spec appears to use only properly structured URIs, so the examples and tests should reflect that.

resources: [
MCP::Resource.new(
uri: "test_resource",
uri: "example://test_resource",
Copy link
Member

@koic koic Jul 27, 2025

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.


@resource = Resource.new(
uri: "test_resource",
uri: "test://test_resource",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@koic
Copy link
Member

koic commented Jul 27, 2025

Can you also update the usage of example:// and test:// in the PR accordingly?

@andyw8 andyw8 force-pushed the andyw8/use-valid-syntax-for-uri-examples branch 3 times, most recently from e31a881 to f4697ac Compare July 27, 2025 15:06
@koic
Copy link
Member

koic commented Jul 27, 2025

This update looks good. Can you squash your commits into one?

@andyw8 andyw8 force-pushed the andyw8/use-valid-syntax-for-uri-examples branch 2 times, most recently from 090380b to 3181af8 Compare July 27, 2025 15:28
@andyw8 andyw8 force-pushed the andyw8/use-valid-syntax-for-uri-examples branch from 3181af8 to 8b92ce0 Compare July 27, 2025 15:29
Copy link
Contributor

@atesgoral atesgoral left a 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.

@atesgoral atesgoral merged commit db1ae35 into modelcontextprotocol:main Jul 27, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants