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

Make the reporter public #341

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Conversation

another-rex
Copy link
Collaborator

Make the reporter package public so that it can be passed into the public DoScan function.

Comment on lines 61 to 69
func (r *Reporter) PrintResult(vulnResult *models.VulnerabilityResults) error {
switch r.format {
case "json":
return PrintJSONResults(vulnResult, r.stdout)
return output.PrintJSONResults(vulnResult, r.stdout)
case "markdown":
PrintMarkdownTableResults(vulnResult, r.stdout)
output.PrintMarkdownTableResults(vulnResult, r.stdout)
case "table":
PrintTableResults(vulnResult, r.stdout)
output.PrintTableResults(vulnResult, r.stdout)
}
Copy link
Member

Choose a reason for hiding this comment

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

nit:
Since it's public, I think it'd make sense to make Reporter an interface, with individual json/markdown/table/void reporter implementations.

That way, an external program could write/wrap their own logging/formatting outputs.

(Also, it avoids this awkward format checking)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This sounds like a interesting idea, I'll make the changes and push it in a separate PR.

@another-rex another-rex merged commit 8a4f876 into google:main Apr 13, 2023
another-rex added a commit that referenced this pull request Apr 26, 2023
The reporter has been refactored to be an interface with various
implementations. Based on the idea from Michael here:
#341 (comment)

Want to get some feedback on whether this is worth changing or not.

I'm also not sure if we should have the different reporter types public
here, or make them private and have the users of the library provide
their own implementations of Reporter.
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.

4 participants