fix(rsr-verify): accept .adoc — the docs exist, the check named .md - #71
fix(rsr-verify): accept .adoc — the docs exist, the check named .md#71hyperpolymath wants to merge 4 commits into
Conversation
rsr-verify.sh fails 7 documentation checks in a plugin whose documentation is COMPLETE. It checks docs/ARCHITECTURE.md, API_REFERENCE.md, FAQ.md, QUICKSTART.md, TROUBLESHOOTING.md, EXAMPLES.md and MIGRATION.md; the plugin ships all seven as .adoc, plus CITATIONS.adoc and COMPATIBILITY.adoc. The .md -> .adoc migration moved the documents and left the verifier naming the old extension, so seven RSR compliance checks have been failing on a compliant plugin. Fixed by accepting either extension - the same pattern the RSR template's own quality.yml uses for README, LICENSE and CONTRIBUTING. The check label drops the extension too, so a failure names the document rather than a filename that may legitimately be either form. Verified: 7/7 checks satisfied against the files actually on disk, 0 missing, shellcheck -S error reports 0 findings. ⚠ I first filed these as MISSING documentation in hyperpolymath/standards#653 and was about to author 14 replacement files. That was wrong: my classifier looked for the .adoc twin at the REPO ROOT, but these live at asdf-ghjk/docs/. Issue #653 is being corrected.
|
|
Up to standards ✅🟢 Issues
|
|
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 (1)
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. (25)
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe verification scripts now accept Markdown or AsciiDoc files for the named documentation checks. ChangesDocumentation validation
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This localized change makes the documentation verifier accept either .md or .adoc files without changing product runtime behavior; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 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 |
![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
While the PR successfully expands the compliance script to support AsciiDoc documentation, it introduces a logic bug in the security verification. Specifically, the 'HTTPS-only downloads' check is rendered ineffective by a || true suffix, creating a false sense of security.
The overall codebase quality remains 'Up to Standards', but the script rsr-verify.sh is duplicated in two separate paths. This duplication, combined with inconsistencies in how root-level files (like README) are handled compared to files in the docs/ directory, suggests the migration to extension-agnostic checks is incomplete. Addressing the security logic and consolidating the scripts is recommended before merging.
About this PR
- The documentation logic for 'either .md or .adoc' has not been applied to root-level files (README, LICENSE, CONTRIBUTING), despite the PR description suggesting alignment with general RSR patterns. Consider applying the same flexible extension logic to these checks for consistency.
- The check for 'SECURITY.adoc' is currently duplicated on line 68 (Documentation category) and line 94 (Security category). It is better to consolidate this into a single check or ensure the labels are distinct if they serve different purposes.
3 comments outside of the diff
asdf-augmenters/asdf-ghjk/scripts/rsr-verify.sh
line 1🟡 MEDIUM RISK
Suggestion: The script 'rsr-verify.sh' appears to be duplicated in two paths ('asdf-augmenters/asdf-ghjk/scripts/' and 'asdf-ghjk/scripts/'). If these scripts are intended to be identical, consider consolidating them into a single shared file or using a symbolic link to prevent maintenance issues and logic desynchronization.
asdf-ghjk/scripts/rsr-verify.sh
line 66⚪ LOW RISK
Nitpick: Update the check name to remove the '.adoc' suffix for consistency with the rest of the documentation checklist.check "CODE_OF_CONDUCT exists" "[[ -f CODE_OF_CONDUCT.adoc ]]"
line 98🟡 MEDIUM RISK
The|| truelogic bypasses the security check, making it effectively always pass regardless of the actual grep results. Remove the bypass so the script correctly identifies non-compliant download scripts.check "HTTPS-only downloads" "grep -q 'https://' bin/download && ! grep -q 'http://' bin/download"
Test suggestions
- Verify documentation check passes when only .md files are present in the docs/ directory.
- Verify documentation check passes when only .adoc files are present in the docs/ directory.
- Verify documentation check fails when neither .md nor .adoc files are present for a required category.
- Confirm that check failure messages for these documentation files do not include file extensions in the label.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify documentation check passes when only .md files are present in the docs/ directory.
2. Verify documentation check passes when only .adoc files are present in the docs/ directory.
3. Verify documentation check fails when neither .md nor .adoc files are present for a required category.
4. Confirm that check failure messages for these documentation files do not include file extensions in the label.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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>
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
rsr-verify.shfails 7 documentation checks in a plugin whose documentation is complete.Every document exists. The
.md→.adocmigration moved them and left the verifier naming the old extension, so 7 of the RSR compliance checks have been failing on a compliant plugin.Fix
Accept either extension — the same pattern the RSR template's own
quality.ymlalready uses for README, LICENSE and CONTRIBUTING:The check label drops the extension too, so a failure message names the document rather than a filename that may be either form.
Verified
shellcheck -S errorProvenance
Found by an estate-wide sweep of 5,111 scripts across 375 repos. Same defect class as the 56 unsatisfiable checks repointed across 18 repos earlier in this campaign (see hyperpolymath/Axiom.jl#82).
⚠ I initially filed these as missing documentation in hyperpolymath/standards#653 and was about to author 14 replacement files. That was wrong — my classifier looked for the
.adoctwin at the repo root, but these live atasdf-ghjk/docs/, so it reported no twin. Issue #653 is being corrected.