Skip to content

fix: new clippy warnings #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 29, 2025
Merged

fix: new clippy warnings #64

merged 1 commit into from
Jun 29, 2025

Conversation

Vaiz
Copy link
Contributor

@Vaiz Vaiz commented Jun 28, 2025

📌 Summary

This PR fixes new clippy warnings. I also had to enable at least one schema by default, but I think it's a good change, and makes it easier to use the crate.

Sorry for mixed PR, these other issues were causing troubles when I tried to fix clippy warnings.

🔍 Related Issues

In PR #63 , ci is failing because of these new errors

  • Fixes #

✨ Changes Made

- added rust-mcp-sdk as dev-dependency to rust-mcp-macros
- strategically placed #[cfg(any(feature = "2025_03_26", not(feature = "2024_11_05")))] in a few places to fix compilation errors
- rust-mcp-transport is not optional anymore

  • fixed clippy warnings

🛠️ Testing Steps

💡 Additional Notes

@Vaiz
Copy link
Contributor Author

Vaiz commented Jun 28, 2025

Technically, now when all clippy warnings are fixed, I can rollback other changes.
Fill free to merge only c3c2557 commit.

@hashemix
Copy link
Member

Thanks @Vaiz , really appreciate the help 🥇

Sorry for mixed PR, these other issues were causing troubles when I tried to fix clippy warnings.

Yea I noticed, I would prefer to avoid mixing concerns in same PR, there are some unrelated to the clippy warnings , those might be better suited for a separate issue and PR.

would you mind updating pr to have changes related to clippy warnings? (i.e c3c2557)

If you revert other changes and only keep c3c2557 , will we have any build error? 🤔

@Vaiz Vaiz changed the title fix: new cliipy warnings fix: new clippy warnings Jun 29, 2025
@Vaiz
Copy link
Contributor Author

Vaiz commented Jun 29, 2025

Yea I noticed, I would prefer to avoid mixing concerns in same PR, there are some unrelated to the clippy warnings , those might be better suited for a separate issue and PR.

would you mind updating pr to have changes related to clippy warnings? (i.e c3c2557)

done

If you revert other changes and only keep c3c2557 , will we have any build error? 🤔

clippy errors should be gone, but other issues, related to not working combinations of features, will remain

@hashemix
Copy link
Member

hashemix commented Jun 29, 2025

Thanks @Vaiz , I will merge it once you resolve the conflicts

other compilation error you mentioned , it does not happen normally when we run cargo build --workspace I think happens only when we call cargo build --all-targets , that I don't think is a blocker but better to be resolved, perhaps in a separate pr

@Vaiz
Copy link
Contributor Author

Vaiz commented Jun 29, 2025

updated PR.

other compilation error you mentioned , it does not happen normally when we run cargo build --workspace I think happens only when we call cargo build --all-targets , that I don't think is a blocker but better to be resolved, perhaps in a separate pr

I encountered these error when I had tried to run cargo clippy --fix, you should see the same.

Copy link
Member

@hashemix hashemix left a comment

Choose a reason for hiding this comment

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

Looks good!
Thanks so much for your contributions and quick responses! 🥇

@hashemix hashemix merged commit ed76a44 into rust-mcp-stack:main Jun 29, 2025
3 checks passed
@Vaiz Vaiz deleted the clippy branch June 29, 2025 20:38
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.

2 participants