ci: validate all config files and report per-file diagnostics#4
ci: validate all config files and report per-file diagnostics#4Copilot wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: igor-holt <125706350+igor-holt@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| s = str(path) | ||
| return ('.git' in s) or ('node_modules' in s) |
There was a problem hiding this comment.
The ignore() function uses substring matching which could incorrectly filter out legitimate files. For example, a file named project.github-actions.json would be ignored because it contains .git as a substring. Consider using path component matching instead:
def ignore(path: Path) -> bool:
parts = path.parts
return '.git' in parts or 'node_modules' in partsThis ensures only directories named .git or node_modules are excluded, not files containing these strings in their names.
| s = str(path) | |
| return ('.git' in s) or ('node_modules' in s) | |
| parts = path.parts | |
| return '.git' in parts or 'node_modules' in parts |
| for f in list(repo_root.rglob('*.yml')) + list(repo_root.rglob('*.yaml')): | ||
| if ignore(f): |
There was a problem hiding this comment.
The script traverses the directory tree twice to find YAML files - once for *.yml and once for *.yaml. This is inefficient and could be slow in repositories with many files. Consider combining the patterns into a single loop:
for f in repo_root.rglob('*.y*ml'):
if not ignore(f) and f.suffix in ('.yml', '.yaml'):
validate_yaml(f)Or collect all files in one pass:
yaml_files = list(repo_root.rglob('*.yml')) + list(repo_root.rglob('*.yaml'))
for f in yaml_files:
if ignore(f):
continue
validate_yaml(f)However, note that the second approach still requires two rglob calls, so the first pattern-based approach is more efficient.
| for f in list(repo_root.rglob('*.yml')) + list(repo_root.rglob('*.yaml')): | |
| if ignore(f): | |
| for f in repo_root.rglob('*.y*ml'): | |
| if ignore(f) or f.suffix not in ('.yml', '.yaml'): |
The validate-configs job exits on first invalid JSON and prints no per-file diagnostics, causing ci-success to fail with "Validate Configs: failure" (job 55250260810).
Changes
✓ pathor✗ path: errorper fileExample output
CI logs will now show which files are invalid. Follow-up PR needed to fix the invalid files.
Original prompt
This pull request was created as a result of the following prompt from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.