fix(scripts): repoint checks at the .adoc files that exist - #84
Conversation
The .md -> .adoc documentation migration moved these files but never updated
the scripts that READ them, so every check naming a .md has been operating on
a file that no longer exists.
Repointed: CHANGELOG.md->CHANGELOG.adoc CODE_OF_CONDUCT.md->CODE_OF_CONDUCT.adoc CONTRIBUTING.md->CONTRIBUTING.adoc docs/PLUGIN_DEVELOPMENT.md->docs/PLUGIN_DEVELOPMENT.adoc docs/QUICKSTART.md->docs/QUICKSTART.adoc docs/TPCF.md->docs/TPCF.adoc MAINTAINERS.md->MAINTAINERS.adoc PROJECT_SUMMARY.md->PROJECT_SUMMARY.adoc README.md->README.adoc SECURITY.md->SECURITY.adoc
Three failure modes were in play across the estate, all fixed by the same
change:
* hard fail - 'check "X.md exists" "[ -f X.md ]"' can never pass
* wrong score - '[ -f X.md ] && ((doc_score++))' silently scores lower
* SILENT SKIP - 'if [ -f X.md ]; then ...greps... fi' skips the whole block,
so the checks inside never run and the gate reports success
by not checking at all
Human-readable labels are repointed too, so failure messages name the file that
is actually inspected. Where a script did 'git add ... X.md', that is fixed as
well - it would have failed at release time.
Only tokens whose .adoc twin exists in this repository were rewritten; anything
without a twin was left untouched for separate triage.
Found by an estate-wide sweep of 454 repos: 56 such checks across 18 repos.
Same defect class as hyperpolymath/Axiom.jl#82.
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe release script now uses ChangesAsciiDoc script updates
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized script update restores documentation checks and release staging to the migrated .adoc files without introducing an actionable merge-blocking risk; it is merge-ready after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. ✨ Finishing Touches📝 Generate docstrings
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Up to standards ✅🟢 Issues
|
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
This PR successfully updates the release and verification scripts to accommodate the migration from Markdown to AsciiDoc (.adoc) files. Codacy analysis indicates the changes are up to standards, but there are functional issues in the scripts that need addressing.
A critical issue exists in scripts/release.sh where the Cargo.lock file is updated during the build but omitted from the git staging command. This inconsistency will likely cause CI failures in environments enforcing locked dependencies. Additionally, documentation content validation in scripts/verify-rsr.sh is case-sensitive, which risks failing checks if documentation headers use Title Case. No automated tests were provided to validate these script modifications.
About this PR
- The PR lacks automated tests for the modified shell scripts. Given the critical nature of release and compliance scripts, consider adding unit or integration tests to verify file paths and logic (e.g., grep commands) behave as expected with the new file format.
Test suggestions
- Verify release.sh correctly prompts for and stages CHANGELOG.adoc during the release flow.
- Verify verify-rsr.sh successfully identifies the presence of all required top-level .adoc documentation files.
- Verify verify-rsr.sh correctly performs grep operations on README.adoc and CONTRIBUTING.adoc.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify release.sh correctly prompts for and stages CHANGELOG.adoc during the release flow.
2. Verify verify-rsr.sh successfully identifies the presence of all required top-level .adoc documentation files.
3. Verify verify-rsr.sh correctly performs grep operations on README.adoc and CONTRIBUTING.adoc.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| check "Contribution guidelines clear" "grep -q 'TPCF\|perimeter' CONTRIBUTING.md || grep -q 'contribution' CONTRIBUTING.md" | ||
| check "TPCF documented" "[ -f docs/TPCF.adoc ]" | ||
| check "Perimeter 3 (Community Sandbox) open" "grep -q 'Community Sandbox' docs/TPCF.adoc" | ||
| check "Contribution guidelines clear" "grep -q 'TPCF\|perimeter' CONTRIBUTING.adoc || grep -q 'contribution' CONTRIBUTING.adoc" |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: Documentation checks should be case-insensitive for better reliability. Update line 108 of scripts/verify-rsr.sh to use 'grep -qi' for both pattern checks in the 'Contribution guidelines clear' validation.
Co-authored-by: codacy-production[bot] <61871480+codacy-production[bot]@users.noreply.github.com> Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
1 participant
The
.md→.adocdocumentation migration moved these files but never updated the scripts that read them, so every check naming a.mdhas been operating on a file that no longer exists.Repointed: CHANGELOG.md->CHANGELOG.adoc CODE_OF_CONDUCT.md->CODE_OF_CONDUCT.adoc CONTRIBUTING.md->CONTRIBUTING.adoc docs/PLUGIN_DEVELOPMENT.md->docs/PLUGIN_DEVELOPMENT.adoc docs/QUICKSTART.md->docs/QUICKSTART.adoc docs/TPCF.md->docs/TPCF.adoc MAINTAINERS.md->MAINTAINERS.adoc PROJECT_SUMMARY.md->PROJECT_SUMMARY.adoc README.md->README.adoc SECURITY.md->SECURITY.adoc
Three failure modes were in play across the estate, all fixed by the same change:
check "X.md exists" "[ -f X.md ]"[ -f X.md ] && ((doc_score++))if [ -f X.md ]; then …greps… fiLabels are repointed too, so failure messages name the file actually inspected. Where a script did
git add … X.md, that is fixed as well — it would have failed at release time.Only tokens whose
.adoctwin exists here were rewritten; anything without a twin was left for separate triage.Found by an estate-wide sweep of 454 repos: 56 such checks across 18 repos. Same class as hyperpolymath/Axiom.jl#82.