Skip to content

Remove pre-1.0 compatibility shims - #222

Merged
thomhurst merged 4 commits into
mainfrom
issue-216-remove-dead-api
Aug 10, 2026
Merged

thomhurst merged 4 commits into
mainfrom
issue-216-remove-dead-api

Conversation

@thomhurst

@thomhurst thomhurst commented Aug 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • remove hidden RespireOptions and DI builder compatibility aliases
  • remove dead string command forwarders from IRespireClient and RespireClient
  • compact RespireCommandFlags.NoRedirect to bit 0
  • migrate first-party tests to current API names

Test plan

  • Respire.Tests (550 passed)
  • Respire.Extensions.Caching.Tests (76 passed)
  • dotnet build Respire.slnx -c Release (local agent guard stopped at 3072 MB after core projects compiled; CI will run full build)

Dependency

README alias cleanup is in #221. Issue #216 is linked as blocked by #218 to preserve merge order.

Closes #216

Summary by CodeRabbit

  • Breaking Changes

    • Removed legacy configuration aliases for cluster mode, Sentinel naming, and response timeouts.
    • Removed string-based command execution overloads; use command-based APIs instead.
    • Updated the NoRedirect flag value for consistency.
  • Documentation

    • Updated Sentinel configuration guidance to use the current property name.
  • Tests

    • Updated coverage for the current configuration and command APIs.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@thomhurst, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b01044f5-432a-4195-a99a-f1036e72ebba

📥 Commits

Reviewing files that changed from the base of the PR and between 3cb891e and ec9d6ca.

📒 Files selected for processing (1)
  • website/docs/fundamentals/connections.md
📝 Walkthrough

Walkthrough

The PR removes obsolete string command overloads and configuration aliases. It changes NoRedirect to flag value 1. It updates benchmarks, tests, and Sentinel documentation to use the current option names.

Changes

API hygiene

Layer / File(s) Summary
Command API cleanup
src/Respire/IRespireClient.cs, src/Respire/RespireClient.cs, src/Respire/RespireCommandFlags.cs, tests/Respire.Extensions.Caching.Tests/RespireDistributedCacheTests.cs, tests/Respire.Tests/Networking/CommandCatalogTests.cs
The string-based execution overloads are removed. RespireCommand overloads remain. NoRedirect now has value 1.
Configuration option migration
src/Respire.Extensions.DependencyInjection/RespireOptionsBuilder.cs, src/Respire/RespireOptions.cs, tests/Respire.Tests/**, tests/Respire.Extensions.Caching.Tests/ServiceCollectionTests.cs, benchmarks/Respire.Benchmarks/CommandRoutingBenchmarks.cs, website/docs/fundamentals/connections.md
Legacy Cluster, ServiceName, and ResponseTimeout usage is replaced with UseCluster, SentinelPrimaryName, and ConnectionIdleReadTimeout. Tests verify that compatibility aliases are absent.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

A rabbit checks the command line,
Old aliases leave without a sign.
UseCluster guides the way,
NoRedirect finds bit one today.
Clean APIs now hop and shine. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.59% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the removal of pre-1.0 compatibility shims, which is the main change in the pull request.
Linked Issues check ✅ Passed The changes satisfy issue #216 by renumbering NoRedirect and removing the compatibility aliases and string command forwarders.
Out of Scope Changes check ✅ Passed The tests, benchmarks, and documentation updates directly support the API cleanup objectives and introduce no unrelated changes.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-216-remove-dead-api

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net10.0)

ℹ️ Execution Details

  • Framework: net10.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: failure
  • Timestamp: 2026-08-10T22:33:27.482Z

Automated integration testing with Redis containers using TUnit

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net8.0)

ℹ️ Execution Details

  • Framework: net8.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: failure
  • Timestamp: 2026-08-10T22:33:27.966Z

Automated integration testing with Redis containers using TUnit

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b17053fe8b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Respire/RespireOptions.cs Resolved
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net10.0)

ℹ️ Execution Details

  • Framework: net10.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: success
  • Timestamp: 2026-08-10T22:37:28.937Z

Automated integration testing with Redis containers using TUnit

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net8.0)

ℹ️ Execution Details

  • Framework: net8.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: success
  • Timestamp: 2026-08-10T22:38:06.429Z

Automated integration testing with Redis containers using TUnit

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

public string? SentinelPrimaryName

P2 Badge Update the Sentinel guide before removing ServiceName

With this alias gone, the published Sentinel example in website/docs/fundamentals/connections.md:112-123 still initializes ServiceName, so readers compiling that example against this revision receive CS0117 instead of a working client. Migrate that example to SentinelPrimaryName as part of this removal.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

BREAKING CHANGE: Remove hidden option aliases and string command overloads, and renumber NoRedirect to bit 0. Use UseCluster, SentinelPrimaryName, ConnectionIdleReadTimeout, and RespireCommand overloads.

Closes #216
Use SentinelPrimaryName after removing the pre-1.0 ServiceName alias.

Refs #216
@thomhurst
thomhurst force-pushed the issue-216-remove-dead-api branch from af3587e to 3cb891e Compare August 10, 2026 22:44
@thomhurst

Copy link
Copy Markdown
Owner Author

Addressed the latest review-body finding in 3cb891e: the Sentinel guide now uses SentinelPrimaryName. Also rebased onto #223 while preserving its safe interpolated fire-and-forget overload. Validation: Respire.Tests 551 passed; caching tests 76 passed; Docusaurus build passed; benchmark project builds net8/net10.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net10.0)

ℹ️ Execution Details

  • Framework: net10.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: success
  • Timestamp: 2026-08-10T22:46:00.121Z

Automated integration testing with Redis containers using TUnit

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net8.0)

ℹ️ Execution Details

  • Framework: net8.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: success
  • Timestamp: 2026-08-10T22:46:36.345Z

Automated integration testing with Redis containers using TUnit

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3cb891ed2c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@thomhurst

Copy link
Copy Markdown
Owner Author

CI triage: Build and Test (net8.0) failed only in FireAndForget_CancellationStopsWaitingButPreservesWrite because cancellation lost a 47 ms timing race. The independent Run Unit Tests (net8.0) job passed all 549 tests in this same commit, and the changed compatibility-shim/benchmark files do not touch this test path. Treating as a confirmed timing flake and rerunning the failed run once.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@website/docs/fundamentals/connections.md`:
- Line 119: Update the sentence preceding the Sentinel configuration example to
reference SentinelPrimaryName instead of the removed ServiceName property,
keeping the surrounding documentation unchanged.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 664005f6-9713-49f5-870c-7a3eaeae9352

📥 Commits

Reviewing files that changed from the base of the PR and between 7a91041 and 3cb891e.

📒 Files selected for processing (18)
  • benchmarks/Respire.Benchmarks/CommandRoutingBenchmarks.cs
  • src/Respire.Extensions.DependencyInjection/RespireOptionsBuilder.cs
  • src/Respire/IRespireClient.cs
  • src/Respire/RespireClient.cs
  • src/Respire/RespireCommandFlags.cs
  • src/Respire/RespireOptions.cs
  • tests/Respire.Extensions.Caching.Tests/RespireDistributedCacheTests.cs
  • tests/Respire.Extensions.Caching.Tests/ServiceCollectionTests.cs
  • tests/Respire.Tests/ExceptionErgonomicsTests.cs
  • tests/Respire.Tests/Networking/ClientCoreTests.cs
  • tests/Respire.Tests/Networking/ClusterTests.cs
  • tests/Respire.Tests/Networking/CollectionScanTests.cs
  • tests/Respire.Tests/Networking/CommandCatalogTests.cs
  • tests/Respire.Tests/Networking/RespireConnectionTests.cs
  • tests/Respire.Tests/Networking/SentinelTests.cs
  • tests/Respire.Tests/RespireOptionsTests.cs
  • tests/Respire.Tests/TelemetryTests.cs
  • website/docs/fundamentals/connections.md
💤 Files with no reviewable changes (5)
  • tests/Respire.Extensions.Caching.Tests/RespireDistributedCacheTests.cs
  • src/Respire/RespireOptions.cs
  • src/Respire/RespireClient.cs
  • src/Respire/IRespireClient.cs
  • src/Respire.Extensions.DependencyInjection/RespireOptionsBuilder.cs

@thomhurst

Copy link
Copy Markdown
Owner Author

The failed net8 build job hit unrelated pub/sub timing drift: SubscribeAsync_Cancelled_AbandonsStalledConnectionBeforeReplacement observed three SUBSCRIBE attempts instead of two. This PR only removes compatibility surface, and the independent net8 Unit Tests workflow passed the same head. Re-running the failed job once; if it recurs, it will be tracked as a cross-cutting synchronization bug rather than patched here.

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net10.0)

ℹ️ Execution Details

  • Framework: net10.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: success
  • Timestamp: 2026-08-10T23:03:47.067Z

Automated integration testing with Redis containers using TUnit

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

public bool UseCluster

P2 Badge Update the packaged cluster example to UseCluster

When users copy the Redis Cluster example from the packaged README (README.md:194-203), it still initializes Cluster = true; after this alias is removed, that example no longer compiles. Because Directory.Build.props includes this README in every package, update the example to use UseCluster in the same change or ensure the documented dependency lands first.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Integration Test Results (net8.0)

ℹ️ Execution Details

  • Framework: net8.0
  • Test Runner: TUnit
  • Runner: Linux - X64
  • Status: success
  • Timestamp: 2026-08-10T23:04:21.306Z

Automated integration testing with Redis containers using TUnit

@thomhurst
thomhurst merged commit a7c7499 into main Aug 10, 2026
20 checks passed
@thomhurst
thomhurst deleted the issue-216-remove-dead-api branch August 10, 2026 23:08

This branch was previously deployed

1 inactive deployment
Pull Requests — ec9d6ca9 Deployed Aug 10, 2026 by thomhurst via modularpipeline #763
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.

API hygiene: RespireCommandFlags bit gap, hidden EditorBrowsable aliases, dead abstract ExecuteAsync string overloads

1 participant