fix: #161: replace prints with logs#772
fix: #161: replace prints with logs#772ramonpetgrave64 wants to merge 3 commits intoslsa-framework:mainfrom
Conversation
| } | ||
|
|
||
| builderID = outBuilderID | ||
| fmt.Fprintf(os.Stderr, "Verifying npm package %s: PASSED\n\n", tarball) |
There was a problem hiding this comment.
where does slog write the result? We need to ensure compatibility and ensure it's written to stderr when fmt.Fprintf(os.Stderr. Otherwise folks won't be able to pipe the result into jq or other tools: jq will pick not just the printed provenance, but the other messages too.
I think before changing this API we should define what the interface is for our logger. And we should provide an interface as logger, so that callers can customize how they log. Here's a fast logging library https://github.com/uber-go/zap, among others, that would need to be instantiable by callers
There was a problem hiding this comment.
I confirmed that by default slog will send to stderr. And then I removed the changes against fmt.Fprintf(os.Stdout, ... statements.
I'll look into how to best allow caller-provided loggers.
There was a problem hiding this comment.
FWIW, I'm personally in favor of using the stdlib logging packages rather than including a dependency.
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
784ebdb to
ff20e18
Compare
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
ff20e18 to
af32d96
Compare
Addresses #161, and followup to #768
This PR replaces all
fmtprint statements withlog/sloglogs at appropriate levels.for successes
and for failures
If you're using slsa-verifier as a library, you can suppress INFO logs by invoking