Skip to content

🌱 Format the JSON output #2251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

🌱 Format the JSON output #2251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c61bb6
:seedling: Format the JSON output
naveensrinivasan Sep 9, 2022
e1dd80e
Included the JSON formatting for cron
naveensrinivasan Oct 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cron/internal/format/json.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ type jsonScorecardResultV2 struct {
// AsJSON exports results as JSON for new detail format.
func AsJSON(r *pkg.ScorecardResult, showDetails bool, logLevel log.Level, writer io.Writer) error {
encoder := json.NewEncoder(writer)
encoder.SetIndent("", " ")
Copy link
Member

@spencerschrock spencerschrock Oct 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

If I change the BQ will it be an issue?

As far as I know, yes. BQ expects newline delimited JSON and refers to the linked spec.

"Each Line is a Valid JSON Value". I can try to confirm with a test upload, but I expect this to break the BQ transfer. Yeah this breaks BQ transfer:

Error while reading data, error message: Failed to parse JSON: Unexpected end of string; Unexpected end of string;

Copy link
Member

Choose a reason for hiding this comment

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

The files in the API bucket don't get transferred as far as I can tell. Would a pretty-print argument (bool?) be acceptable?


out := jsonScorecardResult{
Repo: r.Repo.Name,
Expand Down
1 change: 1 addition & 0 deletions cron/internal/format/json_raw_results.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ func fillJSONRawResults(r *jsonScorecardRawResult, raw *checker.RawResults) erro
// AsRawJSON exports results as JSON for raw results.
func AsRawJSON(r *pkg.ScorecardResult, writer io.Writer) error {
encoder := json.NewEncoder(writer)
encoder.SetIndent("", " ")
Copy link
Member

Choose a reason for hiding this comment

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

same as above comment

out := jsonScorecardRawResult{
Repo: jsonRepoV2{
Name: r.Repo.Name,
Expand Down
2 changes: 2 additions & 0 deletions pkg/json.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ type JSONScorecardResultV2 struct {
// AsJSON exports results as JSON for new detail format.
func (r *ScorecardResult) AsJSON(showDetails bool, logLevel log.Level, writer io.Writer) error {
encoder := json.NewEncoder(writer)
encoder.SetIndent("", " ")

out := jsonScorecardResult{
Repo: r.Repo.Name,
Expand Down Expand Up @@ -125,6 +126,7 @@ func (r *ScorecardResult) AsJSON2(showDetails bool,
}

encoder := json.NewEncoder(writer)
encoder.SetIndent("", " ")
Copy link
Contributor

Choose a reason for hiding this comment

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

There are tools like BigQuery transfers that expect the JSON output to be on a single line. I'm not sure it's our job to prettify the result (breaking change). I fully agree we need more human-friendly results for the badges; but is this the right place for the fix?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a start. @azeemshaikh38 will this cause breaking charges for BigQuery?

Copy link
Contributor

@laurentsimon laurentsimon Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

For BQ we JSONify the results in a different file (under cron/). Some users may be using this output, though.
If you only want these changes for BQ data, I think you need to update cron/ and the GH Action instead, not this file.

I would be OK with that since it's not exposed publicly.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1. Very likely that some users get broken by this.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, If that is the case. I will close it 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

So @azeemshaikh38 If I change the BQ will it be an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

@azeemshaikh38 We discussed this in our bi-weekly. We agreed formatting the JSON output shouldn't be an issue.

out := JSONScorecardResultV2{
Repo: jsonRepoV2{
Name: r.Repo.Name,
Expand Down