add template checker to CI #419
Conversation
mvadari
force-pushed
the
template-checker
branch
from
05b1c69 to
bbf5bd6
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds a template checker to CI that validates Amendment category XLS documents against the AMENDMENT_TEMPLATE.md structure. The changes include new validation scripts, site generation infrastructure with HTML templates and CSS assets, and updated CI workflows to run the new validation checks.
Key changes:
- Introduces
validate_amendment_template.pyscript to check Amendment specs for required sections and placeholder text - Adds
xls_parser.pyto extract and validate metadata from XLS documents - Sets up static site generation infrastructure with HTML templates, CSS assets, and build scripts
Reviewed changes
Copilot reviewed 3 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/validate_amendment_template.py | New validation script that checks Amendment specs for template compliance, including section structure and placeholder detection |
| scripts/xls_parser.py | New parser script that extracts and validates XLS document metadata from markdown files |
| .github/workflows/validate-xls.yml | Updated paths from site/ to scripts/ and added new job to run amendment template validation |
| .github/workflows/deploy.yml | Updated paths from site/ to scripts/ for consistent directory structure |
| scripts/templates/index.html | New template for the main index page with sortable standards table and category navigation |
| scripts/templates/category.html | New template for category-filtered pages with sortable table |
| scripts/templates/contribute.html | New template for the contribution guidelines page |
| scripts/templates/xls.html | Whitespace-only formatting fix |
| scripts/templates/base.html | Whitespace-only formatting fix |
| scripts/assets/style.css | New CSS file with XRPL.org themed styling for the site |
| scripts/assets/favicon.ico | New favicon asset for the site |
| scripts/build_site.py | Whitespace-only formatting fixes |
| scripts/requirements.txt | Whitespace-only formatting fixes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if re.search(pattern, line): | ||
| self.errors.append(ValidationError( | ||
| str(self.file_path), line_num, | ||
| f"Found template placeholder text: {pattern}" |
There was a problem hiding this comment.
The error message shows the raw regex pattern, which will be confusing to users. For example, a pattern like r'\[STypeName\]' would be displayed literally in the error. Consider extracting the actual matched text from the line and showing that instead, or providing a more user-friendly description of what placeholder was found.
| if re.search(pattern, line): | |
| self.errors.append(ValidationError( | |
| str(self.file_path), line_num, | |
| f"Found template placeholder text: {pattern}" | |
| match = re.search(pattern, line) | |
| if match: | |
| placeholder_text = match.group(0) | |
| self.errors.append(ValidationError( | |
| str(self.file_path), line_num, | |
| f"Found template placeholder text: {placeholder_text}" |
| # Get all direct subsections (level 3) for this parent | ||
| parent_num = parent_section.number | ||
| subsections = [ | ||
| s for s in self.sections | ||
| if s.number.startswith(parent_num + ".") and s.level == 3 | ||
| ] |
There was a problem hiding this comment.
The subsection filtering logic may not work correctly for subsections with deeper nesting. The condition s.number.startswith(parent_num + ".") will match both direct children (e.g., "1.1") and deeper descendants (e.g., "1.1.1", "1.1.2"). While the s.level == 3 check helps filter for direct children, if a spec has subsections at level 4 or deeper that start with the parent number, this could still cause issues. Consider using a more precise check that ensures only direct children are included.
| changed_files = result.stdout.strip().split('\n') | ||
|
|
||
| # Filter for XLS README.md files |
There was a problem hiding this comment.
If the git diff returns an empty stdout, calling split('\n') will create a list with one empty string element ['']. This will cause line 367 to evaluate an empty string which will incorrectly pass the startswith('XLS-') check. Consider handling empty output explicitly or filtering out empty strings.
| changed_files = result.stdout.strip().split('\n') | |
| # Filter for XLS README.md files | |
| raw_changed_files = result.stdout.strip().split('\n') if result.stdout else [] | |
| # Filter out empty strings and then filter for XLS README.md files | |
| changed_files = [f for f in raw_changed_files if f] |
| if '<pre>' in line: | ||
| in_metadata = True | ||
| elif '</pre>' in line and in_metadata: | ||
| in_metadata = False |
There was a problem hiding this comment.
Variable in_metadata is not used.
Co-authored-by: Copilot <[email protected]>
1 participant
No description provided.