Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Add 'keyring list' command to CLI #3331
feat: Add 'keyring list' command to CLI #3331
Changes from 27 commits
227365a
80f0661
76273ae
c8f70a1
462ee5f
942ae7f
169fbd1
f6a0eb8
5afefe6
5a6078b
563abbb
0872e94
f9c395f
96dd91f
94ca9d1
50c9fa9
12aee96
4d8fea4
5a48789
6d103ce
662db5a
0b3a9b5
64a999c
1313d54
9371602
4131a0a
c16d843
bcb5df8
d7e407f
b78eacf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 34 in cli/keyring_list.go
Codecov / codecov/patch
cli/keyring_list.go#L33-L34
Check warning on line 39 in cli/keyring_list.go
Codecov / codecov/patch
cli/keyring_list.go#L38-L39
Check warning on line 44 in cli/keyring_list.go
Codecov / codecov/patch
cli/keyring_list.go#L42-L44
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.
nitpick: Not a fan of descriptive comments more than a handful of words being located on the same line as code.
Check warning on line 72 in keyring/file.go
Codecov / codecov/patch
keyring/file.go#L71-L72
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.
suggestion: We tend to prefer using
require
instead ofassert
but regardless of which one you chose, you could replaceassert.Error
andassert.Equal
withassert.ErrorIs
(orrequire.ErrorIs
).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.
Still a bit of growing pains getting used to writing Go! But I agree with this suggestion (both parts.)
Making this change.