-
Notifications
You must be signed in to change notification settings - Fork 64
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
fix: warning message is printed to stdout by CLI #1650
Conversation
Signed-off-by: Susan Shi <[email protected]>
Signed-off-by: Susan Shi <[email protected]>
Signed-off-by: Susan Shi <[email protected]>
Signed-off-by: Susan Shi <[email protected]>
Signed-off-by: Susan Shi <[email protected]>
Signed-off-by: Susan Shi <[email protected]>
Signed-off-by: Susan Shi <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
todo: Add issue for improve unit test for cli |
Can you add the reference to notation CLI test in the issue? https://github.com/notaryproject/notation/tree/main/cmd/notation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Signed-off-by: Susan Shi <[email protected]> Signed-off-by: akashsinghal <[email protected]>
Signed-off-by: Susan Shi <[email protected]>
Description
What this PR does / why we need it:
Currently we are printing warning log out to stdout which pollutes the json verification output.
This PR contains following update:
Which issue(s) this PR fixes (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when the PR gets merged):Fixes #1565
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Validated the output only contains the verification result json. The warning is now logged ot stderr.
Checklist:
Post Merge Requirements
Helm Chart Change