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 3 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
|
|
Warning Review limit reachedNext included review available in 17 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. (3)
|
| Layer / File(s) | Summary |
|---|---|
Markdown and AsciiDoc checks asdf-augmenters/asdf-ghjk/scripts/rsr-verify.sh, asdf-ghjk/scripts/rsr-verify.sh |
The checks for Architecture, API Reference, FAQ, Quickstart, Troubleshooting, Examples, and Migration documentation accept .md or .adoc files. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Merge Risk: ⚪ Minimal · up to 04a94
This is a localized verifier fix that accepts the documented .md or .adoc formats without changing the documents themselves; no actionable merge-blocking risk remains after normal checks and review.
Suggested reviewers: metadatastician
Poem
A rabbit checks each document door
Markdown hops, AsciiDoc follows more
Seven names pass the gate
Two scripts now validate
Neat paths bloom in the burrow floor
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarises the main change: rsr-verify.sh now accepts .adoc documentation files. |
| Description check | ✅ Passed | The description directly explains the verifier defect, the seven documentation checks, the fix, and the verification results. |
| Docstring Coverage | ✅ Passed | 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… |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
Explanation
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 💡 1
🛠️ Fix failing CI checks 💡
- Create stacked PR
- Commit on current branch
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
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 help to get the list of available commands.
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
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>
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.