-
Notifications
You must be signed in to change notification settings - Fork 42
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
chore: update err msg for policy verification #294
Conversation
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.
@qweeah Have you checked the following?
- run
go test -v .
under trustpolicy dir. All tests need to be passed. - run the notation CLI e2e test again, because there are test cases expecting this error message. Changing the error message would fail the e2e test. All e2e tests need to be passed as well.
Will update UT.
Shouldn't that be done when bumping notation-go? If I update e2e spec, then it will fail without the right version of notation-go. |
oh yeah, if you need to change the e2e, you could include the change in this PR: notaryproject/notation#593 because this change is connected closely with your CLI trust policy command. |
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: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
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
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
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
I'm not able to merge this PR due to |
Updated the branch policy to match the workflow defined in |
@patrickzheng200 I bumped notation-go including this change and ran an E2E in my own fork, looks like no test will fail thus no further change required on CLI side. |
This PR rephrased error message returned during trust policy verification.