-
Notifications
You must be signed in to change notification settings - Fork 0
Add documentation checks for every push and PR #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e5f3882
2d0a217
27e38a9
9e48d7e
8066e7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| ## Summary | ||
|
|
||
| <!-- Explain the documentation change and why it is needed. Example: "Update the NCCIA complaint link and clarify the reporting steps." --> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💅 Nitpick Consider providing a more specific example in the summary section to guide contributors. 🤖 WarpFix |
||
|
|
||
| ## Safety and privacy checklist | ||
|
|
||
| <!-- Check every applicable item before requesting review. --> | ||
|
|
||
| - [ ] No victim, client, or active-case data is included | ||
| - [ ] No credentials, financial records, identity numbers, or private media are included | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise The safety and privacy checklist is a valuable addition to ensure compliance and protect sensitive information. 🤖 WarpFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Great addition of a safety and privacy checklist to ensure compliance. 🤖 WarpFix |
||
| - [ ] New factual claims cite an authoritative source | ||
| - [ ] Official links were checked | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Great inclusion of a safety and privacy checklist to ensure sensitive data is not included. 🤖 WarpFix |
||
| - [ ] All added or changed documentation links work | ||
| - [ ] Guidance is defensive and does not promise recovery or legal outcomes | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💅 Nitpick Consider adding a brief explanation for each checklist item for clarity. 🤖 WarpFix |
||
| ## Validation | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Including a validation section is excellent for ensuring that documentation checks are performed. 🤖 WarpFix |
||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💅 Nitpick Consider adding a checklist item for ensuring that all links in the documentation are functional. 🤖 WarpFix |
||
| - [ ] Documentation checks pass | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,113 @@ | ||||||||
| """Validate required documentation and local Markdown links.""" | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Good use of docstrings to explain the purpose of the script. 🤖 WarpFix |
||||||||
|
|
||||||||
| from __future__ import annotations | ||||||||
|
|
||||||||
| import re | ||||||||
| import sys | ||||||||
| from pathlib import Path | ||||||||
| from urllib.parse import unquote | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Using pathlib for file handling improves readability and cross-platform compatibility. 🤖 WarpFix |
||||||||
|
|
||||||||
| ROOT = Path(__file__).resolve().parents[2] | ||||||||
| REQUIRED = ( | ||||||||
| Path("README.md"), | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💅 Nitpick Consider using a constant for the encoding type to avoid magic strings.
Suggested change
🤖 WarpFix |
||||||||
| Path("SECURITY.md"), | ||||||||
| Path("CONTRIBUTING.md"), | ||||||||
| Path("docs/seo-metadata.md"), | ||||||||
|
TehseenTech marked this conversation as resolved.
|
||||||||
| ) | ||||||||
| IGNORED_PARTS = {".git", ".venv", "venv", "env", "node_modules"} | ||||||||
| LINK_PATTERN = re.compile(r"!?\[[^\]]*\]\(([^)]*)\)") | ||||||||
| URI_SCHEME_PATTERN = re.compile(r"^[a-z][a-z0-9+.-]*:", re.IGNORECASE) | ||||||||
|
|
||||||||
|
|
||||||||
| def link_target(raw: str) -> str: | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚨 Critical The script does not handle the case where the required files are missing gracefully. Consider adding error handling.
Suggested change
🤖 WarpFix |
||||||||
| """Return the file component of a Markdown link target.""" | ||||||||
| value = raw.strip() | ||||||||
| if not value: | ||||||||
| return "" | ||||||||
| if value.startswith("<") and ">" in value: | ||||||||
| value = value[1 : value.index(">")] | ||||||||
| else: | ||||||||
| value = value.split(maxsplit=1)[0] | ||||||||
| return unquote(value.split("#", 1)[0].split("?", 1)[0]) | ||||||||
|
|
||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider adding a check to ensure that the 'REQUIRED' files are not empty or contain only comments.
Suggested change
🤖 WarpFix |
||||||||
|
|
||||||||
| def main() -> int: | ||||||||
| """Validate required files and repository-local Markdown links.""" | ||||||||
| errors: list[str] = [] | ||||||||
|
|
||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ensure that the 'REQUIRED' files are always present in the repository to avoid runtime errors. 🤖 WarpFix |
||||||||
| for relative in REQUIRED: | ||||||||
| path = ROOT / relative | ||||||||
| if not path.is_file(): | ||||||||
| errors.append( | ||||||||
| f"missing required file: {relative} " | ||||||||
| "(restore the file or update REQUIRED)" | ||||||||
| ) | ||||||||
|
|
||||||||
| markdown_files = sorted( | ||||||||
| path | ||||||||
| for path in ROOT.rglob("*.md") | ||||||||
| if not any(part in IGNORED_PARTS for part in path.parts) | ||||||||
| ) | ||||||||
|
TehseenTech marked this conversation as resolved.
|
||||||||
| if not markdown_files: | ||||||||
| errors.append("no Markdown files found") | ||||||||
|
|
||||||||
| for path in markdown_files: | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The error handling for reading files could be more specific to differentiate between OSError and UnicodeError.
Suggested change
🤖 WarpFix |
||||||||
| relative = path.relative_to(ROOT) | ||||||||
| try: | ||||||||
| text = path.read_text(encoding="utf-8") | ||||||||
| except (OSError, UnicodeError) as exc: | ||||||||
| errors.append(f"unable to read {relative}: {exc}") | ||||||||
| continue | ||||||||
|
|
||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚨 Critical The script does not handle cases where the Markdown file cannot be read due to permission issues.
Suggested change
🤖 WarpFix |
||||||||
| if not text.strip(): | ||||||||
| errors.append(f"empty Markdown file: {relative}") | ||||||||
| continue | ||||||||
|
|
||||||||
| for line_number, line in enumerate(text.splitlines(), start=1): | ||||||||
| for match in LINK_PATTERN.finditer(line): | ||||||||
| raw = match.group(1).strip() | ||||||||
| if raw.startswith("#") or URI_SCHEME_PATTERN.match(raw): | ||||||||
| continue | ||||||||
|
|
||||||||
| target = link_target(raw) | ||||||||
| if not target: | ||||||||
| errors.append( | ||||||||
| f"{relative}:{line_number}: empty relative link target " | ||||||||
| "(add a path or remove the link)" | ||||||||
| ) | ||||||||
| continue | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚨 Critical The link validation does not account for external links, which could lead to false positives.
Suggested change
🤖 WarpFix |
||||||||
|
|
||||||||
| if target.startswith("/"): | ||||||||
| resolved = (ROOT / target.lstrip("/")).resolve() | ||||||||
| else: | ||||||||
| resolved = (path.parent / target).resolve() | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The link validation logic could be optimized to avoid multiple calls to resolve paths.
Suggested change
🤖 WarpFix |
||||||||
|
|
||||||||
| try: | ||||||||
| resolved.relative_to(ROOT) | ||||||||
| except ValueError: | ||||||||
| errors.append( | ||||||||
| f"{relative}:{line_number}: link escapes repository: " | ||||||||
| f"{raw} (use a path inside the repository)" | ||||||||
| ) | ||||||||
| continue | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The error message for broken links could be more informative by suggesting possible fixes.
Suggested change
🤖 WarpFix |
||||||||
|
|
||||||||
| if not resolved.exists(): | ||||||||
| errors.append( | ||||||||
| f"{relative}:{line_number}: broken relative link: {raw} " | ||||||||
| "(check the target path and filename)" | ||||||||
| ) | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider logging the errors instead of printing them directly for better traceability.
Suggested change
🤖 WarpFix |
||||||||
|
|
||||||||
| if errors: | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚨 Critical The script does not exit with a non-zero status code if no Markdown files are found, which may lead to false positives in CI.
Suggested change
🤖 WarpFix |
||||||||
| print("Documentation checks failed:") | ||||||||
| for error in errors: | ||||||||
| print(f"- {error}") | ||||||||
| return 1 | ||||||||
|
|
||||||||
| print( | ||||||||
| f"Documentation checks passed for {len(markdown_files)} Markdown files." | ||||||||
| ) | ||||||||
| return 0 | ||||||||
|
|
||||||||
|
|
||||||||
| if __name__ == "__main__": | ||||||||
| sys.exit(main()) | ||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,26 @@ | ||||||
| name: Documentation checks | ||||||
|
|
||||||
| on: | ||||||
| push: | ||||||
| pull_request: | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Well-defined triggers for the workflow to ensure documentation checks on relevant events. 🤖 WarpFix |
||||||
| workflow_dispatch: | ||||||
|
|
||||||
| permissions: | ||||||
| contents: read | ||||||
|
|
||||||
| concurrency: | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise The use of concurrency settings is a good practice to optimize workflow execution. 🤖 WarpFix |
||||||
| group: docs-${{ github.workflow }}-${{ github.ref }} | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Well-structured workflow configuration for documentation checks. 🤖 WarpFix |
||||||
| cancel-in-progress: true | ||||||
|
|
||||||
| jobs: | ||||||
| validate: | ||||||
| name: Validate documentation | ||||||
| runs-on: ubuntu-latest | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💅 Nitpick Consider specifying a specific version for the checkout action to avoid unexpected changes.
Suggested change
🤖 WarpFix |
||||||
| timeout-minutes: 5 | ||||||
|
|
||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Specifying a timeout for the job is a good way to prevent hanging processes. 🤖 WarpFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💅 Nitpick Consider specifying a specific version for the checkout action to avoid unexpected changes.
Suggested change
🤖 WarpFix |
||||||
| steps: | ||||||
| - name: Check out repository | ||||||
| uses: actions/checkout@v4 | ||||||
|
|
||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✨ Praise Using the latest version of the checkout action ensures that you benefit from the latest features and fixes. 🤖 WarpFix |
||||||
| - name: Check required files and relative links | ||||||
| run: python .github/scripts/check_docs.py | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✨ Praise
documentationGreat addition of a summary section to clarify the purpose of the documentation changes.
🤖 WarpFix