refactor!: naming & less constrained dependencies #8
Conversation
Cargo.toml
Outdated
| @@ -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.
Cargo.toml
Outdated
| @@ -1,34 +1,31 @@ | |||
| [workspace] | |||
| resolver = "2" | |||
| resolver = "3" | |||
There was a problem hiding this comment.
| resolver = "3" | |
| resolver = "2" |
Cargo.toml
Outdated
| [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
Cargo.toml
Outdated
| [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" } |
Cargo.toml
Outdated
| 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" } |
crates/rust-mcp-macros/Cargo.toml
Outdated
| @@ -8,20 +8,20 @@ repository = "https://github.com/rust-mcp-stack/rust-mcp-sdk" | |||
| documentation = "https://docs.rs/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" |
crates/rust-mcp-sdk/Cargo.toml
Outdated
| @@ -8,19 +8,19 @@ repository = "https://github.com/rust-mcp-stack/rust-mcp-sdk" | |||
| documentation = "https://docs.rs/rust-mcp-sdk" | |||
| 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" |
| @@ -151,10 +151,10 @@ pub trait ServerHandler: Send + Sync + 'static { | |||
| /// | |||
| /// 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( |
crates/rust-mcp-transport/Cargo.toml
Outdated
| @@ -8,17 +8,17 @@ repository = "https://github.com/rust-mcp-stack/rust-mcp-sdk" | |||
| documentation = "https://docs.rs/rust-mcp-transport" | |||
| 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" |
|
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
📌 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.