Kyo Lee would like Owners Override to review this change.
[scripts][shac] Add custom linter rules for skill files.
This adds the `skill_linter` tool to validate and format `SKILL.md`
files. The tool is integrated with SHAC to support automated
formatting via `fx format-code`.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Owners-Override | +1 |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Code-Review | +2 |
# Check for fatal crashes or failure to produce JSON.
if res.retcode != 0 or not res.stdout.strip().startswith("["):
ctx.emit.finding(
level = "error",
message = "Skill linter error:\n%s" % res.stderr,
)
continueNit: I'd rather just remove `raise_on_failure = False` and let shac handle the non-zero exit code.
message = finding.get("message", "Skill linter finding."),The script already sets `message` to a default value if it's not set, so no need for `get()`.
```suggestion
message = finding["message"],
```
logging.info(f"PASSED: '{skill_name}' is valid.")Please fix this WARNING reported by autoreview issue finding: If `fixit` is false and there are validation errors, this branch will execute and unconditionally log `PASSED: '{skill_name}' is valid.` after logging the errors.
Consider guarding this log message so it only prints if the file is genuinely valid:
```python
elif not errors and not warnings and new_content == original_content:
logging.info(f"PASSED: '{skill_name}' is valid.")
```
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
# Check for fatal crashes or failure to produce JSON.
if res.retcode != 0 or not res.stdout.strip().startswith("["):
ctx.emit.finding(
level = "error",
message = "Skill linter error:\n%s" % res.stderr,
)
continueNit: I'd rather just remove `raise_on_failure = False` and let shac handle the non-zero exit code.
Done
message = finding.get("message", "Skill linter finding."),The script already sets `message` to a default value if it's not set, so no need for `get()`.
```suggestion
message = finding["message"],
```
Done
Please fix this WARNING reported by autoreview issue finding: If `fixit` is false and there are validation errors, this branch will execute and unconditionally log `PASSED: '{skill_name}' is valid.` after logging the errors.
Consider guarding this log message so it only prints if the file is genuinely valid:
```python
elif not errors and not warnings and new_content == original_content:
logging.info(f"PASSED: '{skill_name}' is valid.")
```
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
[scripts][shac] Add custom linter rules for skill files.
This adds the `skill_linter` tool to validate and format `SKILL.md`
files. The tool is integrated with SHAC to support automated
formatting via `fx format-code`.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Change has been successfully rolled: https://turquoise-internal-review.googlesource.com/1249410
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |