Skip to content

fix: api ergonomics follow-up - #720

Merged
DaleSeo merged 8 commits into
mainfrom
api-ergonomics
Mar 3, 2026
Merged

fix: api ergonomics follow-up#720
DaleSeo merged 8 commits into
mainfrom
api-ergonomics

Conversation

@DaleSeo

@DaleSeoDaleSeo commented Mar 3, 2026

Copy link
Copy Markdown
Member

Motivation and Context

Just a quick follow-up on PR #715. This update fixes a few issues with the with_* builder methods before we finalize everything at 1.0.

The new methods now accept Option<T> instead of just T. This means you can use .with_title(None), which compiles but doesn't do anything and will trigger a warning from Clippy. It's simpler to just skip the method if you don't have a value. Also, two methods, with_logger and with_content, had the with_ prefix but were actually standalone constructors, which was confusing. The macro code in rmcp-macros has been updated to reflect this change, so it won't call the method if an attribute isn't set.

Since we're already making changes on the way to 1.0, it makes sense to include this now. After the stable release, we would need a whole new major version for what is really just a minor issue.

How Has This Been Tested?

All existing tests pass without modification to their logic.

Breaking Changes

These methods haven't been released yet, so in practice no consumers should be affected.

with_* methods that accepted Option<T> now take T:

// before.with_title(Some("my tool".to_string())).with_stop_reason(Some(reason))// after.with_title("my tool").with_stop_reason(reason)

If you were passing None, just remove the call.

with_logger and with_content are now builder setters instead of constructors:

// beforeLoggingMessageNotificationParam::with_logger(level,"my-logger", data)// afterLoggingMessageNotificationParam::new(level, data).with_logger("my-logger")

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

@DaleSeoDaleSeo self-assigned this Mar 3, 2026
@github-actionsgithub-actionsBot added T-test Testing related changes T-core Core library changes T-macros Macro changes T-model Model/data structure changes labels Mar 3, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review March 3, 2026 16:34
@DaleSeo
DaleSeo requested a review from a team as a code ownerMarch 3, 2026 16:34
@github-actionsgithub-actionsBot added the T-examples Example code changes label Mar 3, 2026
alexhancock
alexhancock previously approved these changes Mar 3, 2026
Comment threadcrates/rmcp-macros/src/tool.rs Outdated
#output_schema_call
#icons_call
#meta_call;
if let Some(__annotations) = #annotations {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why are annotations and execution done this way instead of the same way as title/icons/meta?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Good catch, @alexhancock! Refactored the code so all optional fields follow the same pattern and the generated code is a clean method chain again.

@DaleSeo
DaleSeo merged commit 2d90b76 into mainMar 3, 2026
16 checks passed
@DaleSeo
DaleSeo deleted the api-ergonomics branch March 3, 2026 17:05
@github-actionsgithub-actionsBot mentioned this pull request Mar 3, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-coreCore library changesT-examplesExample code changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@DaleSeo@alexhancock