Skip to content

feat: update notation cert list command output #798

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
Merged
priteshbandi merged 7 commits into from Oct 11, 2023
Merged

feat: update notation cert list command output #798

priteshbandi merged 7 commits into from Oct 11, 2023

Conversation

@ghost
Copy link

@ghost ghost commented Oct 10, 2023 •
edited by ghost
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

Patrick Zheng added 3 commits October 9, 2023 16:54
reformed cert list outputs
0479b37 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
fixed e2e test
3804f31 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
updated column name
457a019 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2023 •
edited by codecov bot
Loading

Codecov Report

Attention: Patch coverage is 42.62295% with 35 lines in your changes missing coverage. Please review.

Project coverage is 64.09%. Comparing base (93e6853) to head (928a0f5).
Report is 143 commits behind head on main.

Files with missing lines Patch % Lines
cmd/notation/cert/list.go 15.78% 30 Missing and 2 partials ⚠️
internal/ioutil/print.go 80.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff            @@
##           main     #798       +/-   ##
=========================================
+ Coverage      0   64.09%   +64.09%     
=========================================
  Files         0       40       +40     
  Lines         0     2284     +2284     
=========================================
+ Hits          0     1464     +1464     
- Misses        0      698      +698     
- Partials      0      122      +122

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

ghost commented Oct 10, 2023 •
edited by ghost
Loading

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

@ghost ghost requested review from toddysm and yizha1 October 10, 2023 02:11
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

FeynmanZhou commented Oct 10, 2023

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
priteshbandi
priteshbandi reviewed Oct 10, 2023
View reviewed changes
yizha1
yizha1 reviewed Oct 11, 2023
View reviewed changes
Patrick Zheng added 2 commits October 11, 2023 10:07
Merge branch 'notaryproject:main' into cert
2b9c5fb
updated per code review
52213ae 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@ghost ghost requested review from priteshbandi and yizha1 October 11, 2023 03:32
@ghost ghost requested review from JeyJeyGao and shizhMSFT October 11, 2023 03:32
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

code clean up
928a0f5 
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@ghost ghost dismissed stale reviews from shizhMSFT, priteshbandi, and yizha1 via 928a0f5 October 11, 2023 07:14
@ghost ghost requested review from priteshbandi and shizhMSFT October 11, 2023 07:17
@ghost
Copy link
Author

ghost commented Oct 11, 2023

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
@ghost ghost deleted the cert branch October 12, 2023 03:25
@shizhMSFT shizhMSFT mentioned this pull request Nov 6, 2023
Merged
6 tasks
@ghost ghost mentioned this pull request Jan 25, 2024
Merged
6 tasks
@chenrui333 chenrui333 mentioned this pull request Jan 28, 2024
Merged
7h3-3mp7y-m4n pushed a commit to 7h3-3mp7y-m4n/notation that referenced this pull request Mar 29, 2025
@7h3-3mp7y-m4n
feat: update notation cert list command output (notaryproject#798)
5cc7aa6 
This PR updates the output format of `notation cert list` command. Resolves notaryproject#609 and notaryproject#772. Example output:
```
STORE TYPE         STORE NAME         CERTIFICATE   
ca                 myStore            myCert.pem    
```

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
FeynmanZhou pushed a commit to FeynmanZhou/notation that referenced this pull request May 15, 2025
@FeynmanZhou
feat: update notation cert list command output (notaryproject#798)
b386e06 
This PR updates the output format of `notation cert list` command. Resolves notaryproject#609 and notaryproject#772. Example output:
```
STORE TYPE         STORE NAME         CERTIFICATE   
ca                 myStore            myCert.pem    
```

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@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

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

1.1.0

Development

Successfully merging this pull request may close these issues.

Use columns with labels for cert list output [Usability Issue] Cert list is not helpful and just lists the files

6 participants

@codecov-commenter @FeynmanZhou @priteshbandi @shizhMSFT @JeyJeyGao @yizha1