Skip to content
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.

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

🌱 Format the JSON output #2251

Closed
wants to merge 2 commits into from
Closed

Conversation

naveensrinivasan
Copy link
Member

@@ -124,6 +124,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

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.

@github-actions
Copy link

Stale pull request message

- This helps with this issue https://github.com/ossf/scorecard/issues/2243

Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
@@ -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
Contributor

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
Contributor

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?

@@ -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
Contributor

Choose a reason for hiding this comment

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

same as above comment

@github-actions
Copy link

Stale pull request message

@spencerschrock
Copy link
Contributor

This PR fell off, but for future attempts I'm not sure this was the right place to implement it. The webapp unmarshalls the file, so I don't think the output in the bucket is what matters.

https://github.com/ossf/scorecard-webapp/blob/e105bd123d601b29b676f18988e3c0a64f8b893b/app/server/get_results.go#L40-L52

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.

None yet

4 participants