refactor!: naming & less constrained dependencies#8
Conversation
hashemix
added
the
🔧 Refactor
| @@ -1,34 +1,31 @@ | |||
| [workspace] | |||
| resolver = "2" | |||
| resolver = "3" | |||
There was a problem hiding this comment.
I noticed a few changes here that seem unrelated to the main issue—those might be better suited for a separate issue and PR. This way, we can review and merge each set of changes independently without mixing concerns.
I will point them out in this PR.
Switching to edition 2024 and resolver 3 are not among changes I would consider at this point until repo is more mature and more tests are added.
| @@ -1,34 +1,31 @@ | |||
| [workspace] | |||
| resolver = "2" | |||
| resolver = "3" | |||
There was a problem hiding this comment.
| resolver = "3" | |
| resolver = "2" |
|
|
||
| [workspace.dependencies] | ||
| # Workspace member crates | ||
| rust-mcp-transport = { version = "0.1.1", path = "crates/rust-mcp-transport" } |
There was a problem hiding this comment.
Need the version there to be able to publish packages from workspace
| [workspace.dependencies] | ||
| # Workspace member crates | ||
| rust-mcp-transport = { version = "0.1.1", path = "crates/rust-mcp-transport" } | ||
| rust-mcp-transport = { path = "crates/rust-mcp-transport" } |
There was a problem hiding this comment.
| rust-mcp-transport = { path = "crates/rust-mcp-transport" } | |
| rust-mcp-transport = { version = "0.1.1", path = "crates/rust-mcp-transport" } |
| rust-mcp-transport = { path = "crates/rust-mcp-transport" } | ||
| rust-mcp-sdk = { path = "crates/rust-mcp-sdk" } | ||
| rust-mcp-macros = { version = "0.1.2", path = "crates/rust-mcp-macros" } | ||
| rust-mcp-macros = { path = "crates/rust-mcp-macros" } |
There was a problem hiding this comment.
| rust-mcp-macros = { path = "crates/rust-mcp-macros" } | |
| rust-mcp-macros = { version = "0.1.2", path = "crates/rust-mcp-macros" } |
| keywords = ["rust-mcp-stack", "model", "context", "protocol", "macros"] | ||
| license = "MIT" | ||
| edition = "2021" | ||
| edition = "2024" |
There was a problem hiding this comment.
Unrelated to the issue and not considering to switch to 2024 edition at this point.
| edition = "2024" | |
| edition = "2021" |
| keywords = ["rust-mcp-stack", "model", "context", "protocol", "sdk"] | ||
| license = "MIT" | ||
| edition = "2021" | ||
| edition = "2024" |
There was a problem hiding this comment.
| edition = "2024" | |
| edition = "2021" |
| /// Default implementation returns method not found error. | ||
| /// Customize this function in your specific handler to implement behavior tailored to your MCP server's capabilities and requirements. | ||
| async fn handle_get_prompt_request( | ||
| async fn handle_prompt_request( |
There was a problem hiding this comment.
For handlers, for consistency, I think it is better to keep the same naming convention that matches the request object name in the official schema. So for GetPromptRequest we would have handle_get_prompt_request()
| async fn handle_prompt_request( | |
| async fn handle_get_prompt_request( |
| keywords = ["rust-mcp-stack", "model", "context", "protocol", "sdk"] | ||
| license = "MIT" | ||
| edition = "2021" | ||
| edition = "2024" |
There was a problem hiding this comment.
| edition = "2024" | |
| edition = "2021" |
hashemix
left a comment
hashemix
left a comment
There was a problem hiding this comment.
Thanks for the improvements!
I added some minor change requests.
Also, I am not sure how feasible this is, but do you think we could avoid breaking changes by having some type aliases for types that are updated 🤔 and deprecate notes for them?
|
I removed the commit with all the |
|
Hi @virtualritz , PR looks great! There’s a merge conflict with the main branch. Would you mind resolving it? Let me know if you need any help. Most conflicts arise from my change in |
|
Done. I don't think type aliases and deprecation warnings are needed at this stage as there are zero listed reverse dependencies on |
hashemix
changed the title
2 participants
📌 Summary
Mostly resolves naming issues.
McpSdkError::AnyError(vsMcpSdkError::AnyErrorStatic) had+ 'static. I believe this was a copypasta oversight and removed it.Cleaned up
Cargo.tomls and reduced dependency versions to major. This way dependent crates can pick up latest, non-API breaking versions, according to semver.🔍 Related Issues
✨ Changes Made
Made
type& getter names adhere to official Rust API guidelines.🛠️ Testing Steps
Ran
cargo test/cargo test --all-features; all green.