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: update notation cert list command output #798

Merged
merged 7 commits into from
Oct 11, 2023
Merged

feat: update notation cert list command output #798

merged 7 commits into from
Oct 11, 2023

Conversation

Two-Hearts
Copy link
Contributor

@Two-Hearts Two-Hearts commented Oct 10, 2023 •
edited
Loading

This PR updates the output format of notation cert list command.
Example output,

STORE TYPE         STORE NAME         CERTIFICATE   
ca                 myStore            myCert.pem    
ca                 myStore            myCert2.pem    
ca                 anotherStore       myCert3.crt

Resolves #609
Resolves #772

@Two-Hearts
reformed cert list outputs
0479b37 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts
fixed e2e test
3804f31 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts
updated column name
457a019 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023 •
edited
Loading

Codecov Report

Merging #798 (52213ae) into main (93e6853) will increase coverage by 64.07%.
The diff coverage is 50.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff            @@
##           main     #798       +/-   ##
=========================================
+ Coverage      0   64.07%   +64.07%     
=========================================
  Files         0       40       +40     
  Lines         0     2285     +2285     
=========================================
+ Hits          0     1464     +1464     
- Misses        0      699      +699     
- Partials      0      122      +122
Files Coverage Δ
cmd/notation/internal/truststore/truststore.go 29.41% <100.00%> (ø)
internal/ioutil/print.go 56.86% <80.00%> (ø)
cmd/notation/cert/list.go 48.95% <20.68%> (ø)

... and 37 files with indirect coverage changes

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

@Two-Hearts Two-Hearts self-assigned this Oct 10, 2023
@Two-Hearts Two-Hearts added the enhancement New feature or request label Oct 10, 2023
@Two-Hearts Two-Hearts added this to the 1.1.0 milestone Oct 10, 2023
@Two-Hearts
Copy link
Contributor Author

Two-Hearts commented Oct 10, 2023 •
edited
Loading

@toddysm @sozercan This PR intends to solve issue #609 and #772. Does the update look good to you? /cc: @yizha1

JeyJeyGao
JeyJeyGao previously approved these changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@JeyJeyGao JeyJeyGao left a comment

Choose a reason for hiding this comment

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

LGTM

@FeynmanZhou
Copy link
Member

The output looks much cleaner and straightforward than before. LGTM

@shizhMSFT shizhMSFT changed the title enhance: update notation cert list command output feat: update notation cert list command output Oct 10, 2023
shizhMSFT
shizhMSFT requested changes Oct 10, 2023
View reviewed changes
internal/ioutil/print.go Show resolved Hide resolved
cmd/notation/internal/truststore/truststore.go Outdated Show resolved Hide resolved
priteshbandi
priteshbandi reviewed Oct 10, 2023
View reviewed changes
internal/ioutil/print.go Show resolved Hide resolved
internal/ioutil/print.go Show resolved Hide resolved
@Two-Hearts
Merge branch 'notaryproject:main' into cert
2b9c5fb
@Two-Hearts
updated per code review
52213ae 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
yizha1
yizha1 previously approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@yizha1 yizha1 left a comment

Choose a reason for hiding this comment

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

Thanks @Two-Hearts

Updates on certificate.md LGTM

priteshbandi
priteshbandi previously approved these changes Oct 11, 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

shizhMSFT
shizhMSFT previously approved these changes Oct 11, 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

@Two-Hearts
code clean up
928a0f5 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@Two-Hearts
Copy link
Contributor Author

Code cleaned up based on @shizhMSFT's suggestions.

shizhMSFT
shizhMSFT approved these changes Oct 11, 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

priteshbandi
priteshbandi approved these changes Oct 11, 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

@priteshbandi priteshbandi merged commit f205e6b into notaryproject:main Oct 11, 2023
5 checks passed
@Two-Hearts Two-Hearts deleted the cert branch October 12, 2023 03:25
@shizhMSFT shizhMSFT mentioned this pull request Nov 6, 2023
Merged
6 tasks
@Two-Hearts Two-Hearts mentioned this pull request Jan 25, 2024
Merged
6 tasks
@chenrui333 chenrui333 mentioned this pull request Jan 28, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@priteshbandi priteshbandi priteshbandi approved these changes

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@yizha1 yizha1 yizha1 left review comments

@gokarnm gokarnm Awaiting requested review from gokarnm gokarnm 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

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

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

Assignees

@Two-Hearts Two-Hearts

Labels
enhancement New feature or request
Projects
None yet
Milestone
1.1.0
7 participants
@Two-Hearts @codecov-commenter @FeynmanZhou @priteshbandi @shizhMSFT @JeyJeyGao @yizha1