Add json output format to check #24
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martinemde
force-pushed
the
martinemde/kkpvkpnwpymy
branch
from
217bb95 to
ef59b76
Compare
martinemde
force-pushed
the
martinemde/srkpxtkmqznx
branch
from
572249d to
270ac28
Compare
ivy
requested changes
Dec 17, 2025
ivy
left a comment
ivy
left a comment
There was a problem hiding this comment.
Looks good to me outside of a design detail: I think we should exit with a non-zero status code when there are violations.
Also suggested a doc comment to point readers/agents to the JSON schema. Love that you added a test that validates against it! 💜
| @@ -0,0 +1,104 @@ | |||
| use itertools::chain; | |||
There was a problem hiding this comment.
Suggested change
| use itertools::chain; | |
| //! JSON output formatter for `pks check -o json`. | |
| //! | |
| //! Serializes check results (violations, stale TODOs, and summary) to JSON. | |
| //! See `schema/check-output.json` for the JSON Schema specification. | |
| use itertools::chain; |
tests/check_test.rs
Outdated
| .arg("-o") | ||
| .arg("json") | ||
| .assert() | ||
| .success() |
tests/check_test.rs
Outdated
| .arg("-o") | ||
| .arg("json") | ||
| .assert() | ||
| .success() |
| csv::write_csv(&result, std::io::stdout())?; | ||
| } | ||
| OutputFormat::JSON => { | ||
| json::write_json(&result, std::io::stdout())?; |
There was a problem hiding this comment.
We should exit with a non-zero status when there are violations:
Suggested change
| json::write_json(&result, std::io::stdout())?; | |
| json::write_json(&result, std::io::stdout())?; | |
| if result.has_violations() { | |
| std::process::exit(1); | |
| } |
I wonder if we should use a different code versus a runtime error? This way, callers don't have to differentiate "runtime error as plain text" versus "violations presented as JSON".
Validate against the included schema for check json output Co-authored-by: Ivy Evans <ivy.evans@gusto.com>
martinemde
force-pushed
the
martinemde/kkpvkpnwpymy
branch
from
ef59b76 to
165a8da
Compare
This follows the convention used by linters like eslint, rubocop, and shellcheck. Builds on clap's existing behavior. All output formats (packwerk, json, csv) behave consistently: - Exit 0 when no violations - Exit 1 when violations found - Exit 2 on internal errors
martinemde
force-pushed
the
martinemde/srkpxtkmqznx
branch
from
270ac28 to
b722534
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change adds a json output option and provides a schema to describe the output format. The goal is to be able to programmatically parse the output of
pks checkfor other tools like danger.We decided to split the idea of a stale todo from the idea of a violation, since even though they talk about the same idea, the have different meanings.
pks update. It's a lint warning.