Skip to content

docs: remove duplicate Testing section, list the semaphore sample#143

Merged
steingran merged 1 commit into
mainfrom
claude/multilock-pr-117-review-1fpxvz
Jul 5, 2026
Merged

docs: remove duplicate Testing section, list the semaphore sample#143
steingran merged 1 commit into
mainfrom
claude/multilock-pr-117-review-1fpxvz

Conversation

@steingran

Copy link
Copy Markdown
Owner

Small README cleanup following the #117 merge. The distributed semaphore functionality itself is already fully documented (use cases, quick start, service usage patterns, per-provider registration, configuration options, semaphore-vs-leader-election comparison) — this fixes two blemishes that #117's README changes introduced:

🤖 Generated with Claude Code

https://claude.ai/code/session_011D79i3saTKrBEKrhpMuDWv


Generated by Claude Code

The semaphore documentation added in #117 introduced a second "## Testing"
section just before Samples, shadowing the fuller pre-existing one that
covers the Docker-based integration tests. Remove the duplicate and add
the new MultiLock.SemaphoreSample to the Samples section with its run
command.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011D79i3saTKrBEKrhpMuDWv
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

docs: remove duplicate Testing section and list semaphore sample in README

📝 Documentation 🕐 Less than 5 minutes

Grey Divider

AI Description

• Remove the duplicate README “Testing” section introduced in #117
• Add MultiLock.SemaphoreSample to the Samples list with a run command example
High-Level Assessment

The PR’s approach is the right fix: keep the more complete existing Testing section and remove the duplicate, while ensuring the new semaphore sample is discoverable in the Samples list with a minimal run snippet.

Files changed (1) +5 / -13

Documentation (1) +5 / -13
README.mdRemove duplicate Testing section and document semaphore sample +5/-13

Remove duplicate Testing section and document semaphore sample

• Deletes the short, duplicated “## Testing” block that was shadowing the more complete testing documentation later in the README. Adds the MultiLock.SemaphoreSample to the Samples list and includes a quick run example showing the expected arguments.

README.md

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Semaphore sample args mismatch 🐞 Bug ≡ Correctness
Description
README documents running the semaphore sample as dotnet run [provider] [maxConcurrent], but the
sample supports an optional third positional argument for provider connection strings and advertises
that in its usage text. This can mislead users into thinking connection strings are not accepted or
that extra args are invalid.
Code

README.md[R809-811]

+# Run the semaphore sample: dotnet run [provider] [maxConcurrent]
+cd samples/MultiLock.SemaphoreSample
+dotnet run inmemory 3
Relevance

⭐⭐⭐ High

Team previously accepted fixing docs/examples to match actual API signature/usage (PR #10).

PR-#10

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The README addition documents only two arguments, but the semaphore sample code clearly accepts and
documents a third argument for connection strings via args[2] and prints a usage string that
includes it.

README.md[786-812]
samples/MultiLock.SemaphoreSample/Program.cs[47-77]
samples/MultiLock.SemaphoreSample/Program.cs[92-98]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The README’s semaphore sample command line usage is incomplete: it mentions only two arguments (`[provider] [maxConcurrent]`), while the sample supports a third optional argument for provider connection strings.

### Issue Context
The sample’s `Program.cs` reads `args[2]` for multiple providers (e.g., sqlserver/redis/postgresql) and prints usage including `[connection_string]`.

### Fix Focus Areas
- README.md[809-811]

### Suggested change
Update the README line to reflect the optional third argument (e.g., `dotnet run [provider] [maxConcurrent] [connectionString]`) and optionally add one example showing a provider that uses it (e.g., `redis` or `sqlserver`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread README.md
Comment on lines +809 to +811
# Run the semaphore sample: dotnet run [provider] [maxConcurrent]
cd samples/MultiLock.SemaphoreSample
dotnet run inmemory 3

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

1. Semaphore sample args mismatch 🐞 Bug ≡ Correctness

README documents running the semaphore sample as dotnet run [provider] [maxConcurrent], but the
sample supports an optional third positional argument for provider connection strings and advertises
that in its usage text. This can mislead users into thinking connection strings are not accepted or
that extra args are invalid.
Agent Prompt
### Issue description
The README’s semaphore sample command line usage is incomplete: it mentions only two arguments (`[provider] [maxConcurrent]`), while the sample supports a third optional argument for provider connection strings.

### Issue Context
The sample’s `Program.cs` reads `args[2]` for multiple providers (e.g., sqlserver/redis/postgresql) and prints usage including `[connection_string]`.

### Fix Focus Areas
- README.md[809-811]

### Suggested change
Update the README line to reflect the optional third argument (e.g., `dotnet run [provider] [maxConcurrent] [connectionString]`) and optionally add one example showing a provider that uses it (e.g., `redis` or `sqlserver`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.73%. Comparing base (2209b95) to head (a3d1a6e).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #143      +/-   ##
==========================================
+ Coverage   72.61%   72.73%   +0.12%     
==========================================
  Files          65       65              
  Lines        4974     4974              
  Branches      608      608              
==========================================
+ Hits         3612     3618       +6     
+ Misses       1156     1153       -3     
+ Partials      206      203       -3     

see 2 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2209b95...a3d1a6e. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@steingran
steingran merged commit f5e3445 into main Jul 5, 2026
7 checks passed
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