Add a migration path for Server::Transports::StdioTransport
#51
Conversation
|
ty! nice example to follow for handling future deprecations more gracefully 🙏🏻 |
|
This is a good way how to handle migrations I think 👍🏻 But I think this does not resolve the actual issue unless a new gem version is released. The actual issue is that the code in the README file examples matches |
|
Yeah, the inconsistency between the product compornent and the documentation was something I was concerned about as well. That said, the essential issue is likely the breaking change with misalignment between the release cycles of the product compornent and the documentation. What this PR does is convert the "breaking change" to the old Releasing this PR early is probably the best option. So, if this PR is merged and released promptly, there would be no need to update the documentation. That would help move users' attention from the old API to the new one. Of course, if the release is still far off, updating the documentation would be the best option. Let's ask about the release plan. cc @atesgoral @topherbullock |
Follow-up to modelcontextprotocol#27. This PR adds a migration path for `Server::Transports::StdioTransport`. e.g., ```console $ cat /tmp/example.rb require "mcp/transports/stdio" MCP::Transports::StdioTransport ``` This change displays the following warning instead of raising an error for such cases, allowing execution to proceed: ```console $ bundle exec ruby /tmp/example.rb /tmp/example.rb:1: warning: Use `require "mcp/server/transports/stdio_transport"` instead of `require "mcp/transports/stdio"`. Also use `MCP::Server::Transports::StdioTransport` instead of `MCP::Transports::StdioTransport`. This API is deprecated and will be removed in a future release. ``` Currently (v0.1.0), it results in an error without any warning. This MCP gem is not yet close to version 1.0, but introducing breaking changes to a published interface is undesirable. This compatibility code should be removed at some point after a few releases. Closes modelcontextprotocol#50.
191ebdd to
16ab4ba
Compare
|
Yes I didn't really want to imply updating the README too or anything. I think the current PR content is enough to handle the problem for users. 👍🏻 Just wanted to point out the fact that merging this PR is not sufficient to actually close the issue. |
|
@koichi Re: Release plan: There's really no release plan. Since we recently made this repo public and opened the API to public scrutiny, I'm happy with rapidly iterating on the API ergonomics, releasing minor versions while making breaking changes, until we hit v1.0.0. I was waiting for some of the pending PRs that change the shape of things to settle before calling a need for 0.2.0. Happy to hear other suggestions, including a more concrete path towards 1.0.0. |
|
@atesgoral Thank you for sharing the release policy. I have two points I'd like to raise:
I think releasing multiple 0.x versions before reaching 1.0.0 is a good approach. To avoid that, it would be better to introduce a deprecation period with warning messages before applying breaking changes. This is a practical approach commonly used in projects like Ruby, Rails, and others. It helps provide a more user-friendly migration path. Also, for deprecated APIs or breaking changes, it's helpful if users can track which release introduced them. Having some form of changelog would be a better practice in that regard. |
|
While the concern about breaking changes is based on the report in issue #50, given that the project is still at version 0.1.0, where rapid iteration is important, and considering that there has only been one release so far, it might not be necessary to be overly cautious about them. That said, it would still be good to have changelog at some point :-) |
|
@koic For uniformity, the TS SDK doesn't have a CHANGELOG, yet they use the Releases feature to document changes. I can see how having a CHANGELOG would help us track the changes to eventually document as we publish a new release. I'm on the fence. Let's still merge this PR and discuss whether we want a CHANGELOG here: #65 |
|
Yeah, Thank you for opening #65! I'll add my current thoughts there later. |
Motivation and Context
Follow-up to #27.
This PR adds a migration path for
Server::Transports::StdioTransport. e.g.,This change displays the following warning instead of raising an error for such cases, allowing execution to proceed:
Currently (v0.1.0), it results in an error without any warning. This MCP gem is not yet close to version 1.0, but introducing breaking changes to a published interface is undesirable.
This compatibility code should be removed at some point after a few releases.
Closes #50.
Types of changes
Checklist