Skip to content

refactor: Remove redundant code and extract magic numbers#1659

Merged
thomhurst merged 2 commits into
mainfrom
fix/code-cleanup
Dec 30, 2025
Merged

refactor: Remove redundant code and extract magic numbers#1659
thomhurst merged 2 commits into
mainfrom
fix/code-cleanup

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • Remove redundant case-sensitive string comparisons in CliScraperBase.ShouldSkipBasedOnHelpText() that are already covered by case-insensitive checks
  • Extract magic number 5000 to named constant ProgressPrinterGracePeriodMs in PrintProgressExecutor

Test plan

  • Both affected projects build successfully
  • No new warnings introduced (existing warnings in codebase are pre-existing)
  • Changes are backwards compatible (no behavior change)

Closes #1622, #1618

🤖 Generated with Claude Code

thomhurst and others added 2 commits December 30, 2025 12:43
Remove redundant Ordinal string comparisons that are already covered
by OrdinalIgnoreCase checks in ShouldSkipBasedOnHelpText method:
- helpText.Contains("deprecated", StringComparison.Ordinal)
- helpText.Contains("experimental", StringComparison.Ordinal)

Closes #1622

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace magic number 5000 with named constant ProgressPrinterGracePeriodMs
in PrintProgressExecutor.DisposeAsync to improve code readability
and maintainability.

Closes #1618

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@thomhurst
Copy link
Copy Markdown
Owner Author

Summary

Removes redundant case-sensitive string checks and extracts magic number to named constant.

Critical Issues

None found ✅

Suggestions

None - both changes are straightforward improvements.

Verdict

APPROVE - No critical issues

Change 1: The redundant case-sensitive checks are correctly identified and removed. StringComparison.OrdinalIgnoreCase already matches "DEPRECATED", "deprecated", "Deprecated", etc., making the additional .Contains("deprecated", StringComparison.Ordinal) redundant. Same logic applies to "experimental".

Change 2: Extracting the magic number 5000 to ProgressPrinterGracePeriodMs improves readability and maintainability. The XML documentation clearly explains its purpose.

@thomhurst thomhurst merged commit ed2aa1d into main Dec 30, 2025
12 checks passed
@thomhurst thomhurst deleted the fix/code-cleanup branch December 30, 2025 12:51
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.

Code smell: CliScraperBase.ShouldSkipBasedOnHelpText has redundant string comparisons

1 participant