Skip to content

Conversation

@BenjaminPelletier
Copy link
Member

This PR furthers #404 by adding programmatic enforcement disallowing new checks to be defined without documented severity, and disallowing documented severity from being removed from existing checks. It also tracks progress toward eliminating checks without documented severity. This was prompted by the new bug fixed in #967.

This is accomplished by tracking all current checks without documented severity in checks_without_severity.json. make lint ensures that all checks without documented severity are listed in this file -- if any checks without documented severity exist that aren't listed in that file, they are treated as new bad checks and make lint fails. When checks without documented severity are removed (by documenting the severity of one or more checks), make format automatically updates checks_without_severity.json. make lint detects when this is necessary and fails with a message to run make format when necessary.

Tested manually via:

  1. make lint -> verify pass with warning about current number of checks without documented severity (622)
  2. make format -> verify no changes, verify message that number of checks without documented severity has not changed
  3. Add severity to check documentation
  4. make lint -> verify fail with message to run make format
  5. make format -> verify changes to checks_without_severity.json, verify message about update
  6. make lint -> verify pass
  7. Revert changes, remove severity from check documentation
  8. make lint -> verify fail with message indicating new check with missing severity documentation
  9. make format -> verify no changes, verify warning message about new checks with undocumented severity

Once there are zero entries in checks_without_severity.json, it should be possible to remove the severity argument from record_failed and thus complete the task in 404. At that point, much of this PR can be reverted/removed, though detection of check documentation without severity will remain permanently.

@BenjaminPelletier BenjaminPelletier marked this pull request as ready for review February 19, 2025 21:21
Copy link
Contributor

@barroco barroco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thank you

@BenjaminPelletier BenjaminPelletier merged commit 0379e3d into interuss:main Feb 24, 2025
20 checks passed
@BenjaminPelletier BenjaminPelletier deleted the track-check-severity branch February 24, 2025 15:55
github-actions bot added a commit that referenced this pull request Feb 24, 2025
Co-authored-by: Michael Barroco <michael@orbitalize.com> 0379e3d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants