Apply PhotoCleaner round-2 feedback: rationale wording, MD033 clarity, CPM property owner - #408
Merged
Conversation
…, CPM property owner Three items from the round-2 convergence (PhotoCleaner#14), adopting the downstream wording: 1. configure.sh cmd_apply rationale reworded so it no longer reads as a run-on - two sentences with the shell option in backticks, still terse. 4. MD033 note made explicit (PhotoCleaner's PR #18 form): HTML comments are permitted (markdownlint does not flag them), HTML elements are flagged, and anything with a native markdown equivalent uses the markdown - the earlier "allowed in comments only" could read as "elements allowed inside comments". 5. CODESTYLE now states Directory.Packages.props owns ManagePackageVersionsCentrally explicitly (in this file, not Directory.Build.props), and a README TODO records the fleet-consistency sweep - PlexCleaner sets the property in Directory.Build.props, off canonical. Item 2 (stale round-2 snapshot at pickup) needs no hub change - the AUDIT.md re-verify-before- acting rule caught it downstream, exactly as designed. Item 3 (CPM section reads as present- state mid-convergence) is inherent to carrying a standard before adopting it; the block is already prescriptive and the transient clears once the repo adopts CPM. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR incorporates round-2 PhotoCleaner feedback by tightening rationale wording in configure.sh, clarifying the intent of markdownlint MD033 guidance, and making CPM ownership explicit in the .NET style section.
Changes:
- Reworded
repo-config/configure.shapply-path rationale comment for clarity. - Clarified
MD033expectations inCODESTYLE.md(HTML comments vs HTML elements, prefer native Markdown). - Documented CPM ownership more explicitly in
CODESTYLE.mdand added a fleet sweep TODO inREADME.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| repo-config/configure.sh | Adjusts explanatory comment around write behavior and validation flow. |
| README.md | Adds a TODO item tracking a fleet-wide CPM-property placement sweep. |
| CODESTYLE.md | Clarifies Markdown linting guidance (MD033) and CPM property ownership in .NET conventions. |
Uh oh!
There was an error while loading. Please reload this page.
…rites The probe reads run inside `if` conditions and are exempt from set -e, so "aborts on any non-zero exit" overstated it. The rationale now says a failed write aborts the script because these writes run unguarded, which is the property that actually holds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
repo-config/configure.sh:125
- Comment grammar: "fail loud" should be "fail loudly".
# The writes below silence stdout only (the success-response JSON is noise). They still fail loud -
# gh errors go to stderr and a failed write aborts the script (these writes run unguarded under `set -e`).
The programmatic insertion left the new backlog bullet ending in LF while README.md is CRLF, which editorconfig-checker rejects. Restore CRLF on that line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
repo-config/configure.sh:125
- Comment grammar: "fail loud" reads ungrammatical here (and earlier review asked for "fail loudly"). Consider switching to the adverb form for clarity.
# The writes below silence stdout only (the success-response JSON is noise). They still fail loud -
# gh errors go to stderr and a failed write aborts the script (these writes run unguarded under `set -e`).
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 23, 2026
ptr727 added a commit
that referenced
this pull request
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three actionable items from the PhotoCleaner round-2 convergence (ptr727/PhotoCleaner#14 round-2 comment), adopting the downstream wording where offered.
Directory.Packages.propsownsManagePackageVersionsCentrallyexplicitly (in that file, notDirectory.Build.props), and a README TODO records the fleet sweep: PlexCleaner sets it inDirectory.Build.props, off canonical.No hub change for the other two: item 2 (stale round-2 snapshot at pickup) — the AUDIT.md re-verify-before-acting rule caught it downstream, as designed; item 3 (CPM section reads as present-state mid-convergence) — inherent to carrying a standard before adopting it, the block is already prescriptive.
Verification
validate.pygreen, shellcheck clean, markdownlint clean, CRLF/LF preserved, no em-dash.🤖 Generated with Claude Code