Repository navigation
Remove Unused Code Ahead of Expanding Test Coverage - #1170
Conversation
Coverage analysis found members with no caller anywhere in the product, plugins, tests, or the sandbox. Testing them would raise the coverage number without testing anything that runs, so they are deleted instead: the typed MediaInfo XML parser and its schema types, a streamed-stdout tool execution path, a media props self-test, and unused command-line builder options. The sidecar v4 to v5 upgrade keeps its generic XML to JSON conversion. Bump the version to 3.24 for the coverage work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 16 minutes. View limit details
|
| Layer / File(s) | Summary |
|---|---|
Remove typed MediaInfo XML parsing PlexCleaner/MediaInfoToolXmlSchema.cs, PlexCleaner/MediaInfoXmlParser.cs, ARCHITECTURE.md |
The typed MediaInfo XML schema and parser methods are removed. The architecture documentation describes GenericXmlToJson() and its use in sidecar upgrades. |
Remove unused tool and media-property APIs PlexCleaner/MediaTool.cs, PlexCleaner/FfProbeBuilder.cs, PlexCleaner/MediaInfoBuilder.cs, PlexCleaner/MkvMergeBuilder.cs, PlexCleaner/ProcessFile.cs |
The stdout-streaming execution method, command-builder methods, and media-property self-test method are removed. |
Update 3.24 release information HISTORY.md, README.md, version.json |
Release information describes the removals and states that processing behavior is unchanged. The version changes from 3.23 to 3.24, and the Xml2CSharp entry is removed from the README. |
Estimated code review effort: 2 (Simple) | ~15 minutes
Change: Other
Possibly related PRs
- ptr727/PlexCleaner#1091: Adds error-recording calls to
ExecuteStreamStdOut, which this change removes.
Suggested labels: chore
Merge Risk: 
Minimal · previous review 6a163
-
The change is mergeable on the available evidence. The remaining documentation clarification concerns pre-existing wording and does not affect parser behavior.
Pre-merge checks: 
5
-
Full details: Title check
-
Explanation
The title "Remove Unused Code Ahead of Expanding Test Coverage" matches the main change. The PR deletes unused product code, including the typed MediaInfo XML parsing chain and its schema file, MediaTool.ExecuteStreamStdOut, ProcessFile.TestMediaProps, and several builder options. The title states the purpose (removal of unused code) and the motivation (future test coverage). It is specific enough for a teammate scanning history to understand the primary change.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
PR Summary by QodoRemove unused media tooling and typed XML parsing code
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
A recorded local strict-review pass covers head |
First step toward 80% code coverage: delete code that nothing calls, rather than write tests for it.
Why
A per-line coverage analysis mapped every uncovered line to the member that holds it, then searched for callers of each fully uncovered member. These had no caller anywhere in the product, the plugins, the tests,
Sandbox/,RegressionTests/, the docs, or the workflows, by name or by string:MediaInfoXmlParser.MediaInfoXmlToJsonand the typed XML chain behind it (MediaInfoFromXml,ParseMediaInfo,ParseMedia,ParseTrack), andMediaInfoToolXmlSchema.cs, which only that chain reached.MediaTool.ExecuteStreamStdOut. OnlyExecuteStreamStdErris used.ProcessFile.TestMediaProps.FfProbeBuilder.LogLevelQuiet/ShowFrames,MediaInfoBuilder.OutputFormatXml, andMkvMergeBuilder.Quiet.GenericXmlToJsonstays, since the sidecar v4 to v5 upgrade uses it.Extensions.LogAndPropagateis also unused but stays, since the carried .NET code style teaches it as the pattern.Changes
version.jsonto 3.24 for the coverage work, and opens the 3.24 release notes in README.md and HISTORY.md.Verification
dotnet format style --verify-no-changesare clean.mainreport exactly. The coverable total drops by the deleted lines, which moves the overall figure from 58.15% to about 59.5%.🤖 Generated with Claude Code
Summary by CodeRabbit