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.

Sign up for GitHub

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

Jump to bottom

feat: add show and import for trust policy management #593

Merged
merged 1 commit into from Mar 30, 2023
Merged

feat: add show and import for trust policy management #593

merged 1 commit into from Mar 30, 2023

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Mar 22, 2023 •
edited

This PR implements what's described in policy.md.

Resolves #548

@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2023 •
edited

Codecov Report

Merging #593 (f84c685) into main (56945b3) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #593      +/-   ##
==========================================
- Coverage   34.45%   34.43%   -0.02%     
==========================================
  Files          32       32              
  Lines        1843     1844       +1     
==========================================
  Hits          635      635              
- Misses       1187     1188       +1     
  Partials       21       21
Impacted Files Coverage Δ
cmd/notation/main.go 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

shizhMSFT
shizhMSFT reviewed Mar 22, 2023
View reviewed changes
cmd/notation/policy/show.go Outdated Show resolved Hide resolved
cmd/notation/policy/show.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
@yizha1
Copy link
Contributor

yizha1 commented Mar 22, 2023

Suggest marking the new policy command as Preview in the output of help command.

byronchien
byronchien reviewed Mar 22, 2023
View reviewed changes
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
test/e2e/suite/command/policy.go Outdated Show resolved Hide resolved
@qweeah
Copy link
Contributor Author

qweeah commented Mar 23, 2023

Suggest marking the new policy command as Preview in the output of help command.

Added Preview feature label. @yizha1 Do notation have or need to have any doc on the classification of work-in-progress feature labels?

@yizha1
Copy link
Contributor

yizha1 commented Mar 23, 2023

Suggest marking the new policy command as Preview in the output of help command.

Added Preview feature label. @yizha1 Do notation have or need to have any doc on the classification of work-in-progress feature labels?

No. We need this kind of doc.

patrickzheng200
patrickzheng200 reviewed Mar 23, 2023
View reviewed changes
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/show.go Outdated Show resolved Hide resolved
@qweeah qweeah mentioned this pull request Mar 23, 2023
Closed
patrickzheng200
patrickzheng200 reviewed Mar 26, 2023
View reviewed changes
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/show.go Outdated Show resolved Hide resolved
patrickzheng200
patrickzheng200 reviewed Mar 27, 2023
View reviewed changes
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/show.go Outdated Show resolved Hide resolved
patrickzheng200
patrickzheng200 previously approved these changes Mar 27, 2023
View reviewed changes
Copy link
Contributor

@patrickzheng200 patrickzheng200 left a comment

Choose a reason for hiding this comment

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

LGTM

@patrickzheng200
Copy link
Contributor

patrickzheng200 commented Mar 27, 2023 •
edited

Lookes like the e2e test has failed.

@patrickzheng200 patrickzheng200 self-requested a review March 27, 2023 07:48
patrickzheng200
patrickzheng200 previously approved these changes Mar 27, 2023
View reviewed changes
Copy link
Contributor

@patrickzheng200 patrickzheng200 left a comment

Choose a reason for hiding this comment

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

LGTM

byronchien
byronchien previously approved these changes Mar 27, 2023
View reviewed changes
qweeah
qweeah commented Mar 28, 2023
View reviewed changes
specs/commandline/policy.md Outdated Show resolved Hide resolved
priteshbandi
priteshbandi reviewed Mar 28, 2023
View reviewed changes
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/import.go Outdated Show resolved Hide resolved
cmd/notation/policy/show.go Outdated Show resolved Hide resolved
@patrickzheng200 patrickzheng200 mentioned this pull request Mar 28, 2023
Merged
shizhMSFT
shizhMSFT previously approved these changes Mar 28, 2023
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

cmd/notation/policy/import.go Outdated Show resolved Hide resolved
shizhMSFT
shizhMSFT previously approved these changes Mar 28, 2023
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

byronchien
byronchien previously approved these changes Mar 28, 2023
View reviewed changes
priteshbandi
priteshbandi previously approved these changes Mar 28, 2023
View reviewed changes
Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM

patrickzheng200
patrickzheng200 previously approved these changes Mar 29, 2023
View reviewed changes
Copy link
Contributor

@patrickzheng200 patrickzheng200 left a comment

Choose a reason for hiding this comment

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

LGTM

@qweeah
feat: add show and import for trust policy management
f84c685 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
Copy link
Contributor Author

qweeah commented Mar 29, 2023 •
edited

Hi @priteshbandi @shizhMSFT @patrickzheng200 I forget to sign the commits.

I re-signed with my GPG key and rebased, can you help review again, thanks in advance.

patrickzheng200
patrickzheng200 approved these changes Mar 29, 2023
View reviewed changes
Copy link
Contributor

@patrickzheng200 patrickzheng200 left a comment

Choose a reason for hiding this comment

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

LGTM

shizhMSFT
shizhMSFT approved these changes Mar 30, 2023
View reviewed changes
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@shizhMSFT shizhMSFT merged commit 0ec3b3d into notaryproject:main Mar 30, 2023
4 checks passed
duffney pushed a commit to duffney/notation that referenced this pull request Mar 30, 2023
@qweeah @duffney
feat: add show and import for trust policy management (notaryproj…
21cf064 
…ect#593)

This PR implements what's described in
[policy.md](https://github.com/notaryproject/notation/blob/18efc3de233ef5eb00e2f32009475a2b8a3eae47/specs/commandline/policy.md).

Resolves notaryproject#548

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Josh Duffney <jduffney@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yizha1 yizha1 yizha1 left review comments

@priteshbandi priteshbandi priteshbandi approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@patrickzheng200 patrickzheng200 patrickzheng200 approved these changes

@byronchien byronchien byronchien left review comments

@gokarnm gokarnm Awaiting requested review from gokarnm gokarnm is a code owner

@JeyJeyGao JeyJeyGao Awaiting requested review from JeyJeyGao JeyJeyGao is a code owner

@justincormack justincormack Awaiting requested review from justincormack

@NiazFK NiazFK Awaiting requested review from NiazFK NiazFK is a code owner

@rgnote rgnote Awaiting requested review from rgnote rgnote is a code owner

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CLI Cmds for trust policy management - phase 1
7 participants
@qweeah @codecov-commenter @yizha1 @patrickzheng200 @priteshbandi @byronchien @shizhMSFT