[keychain]: add functionality for container registry list command #502
Conversation
jglogan
left a comment
jglogan
left a comment
There was a problem hiding this comment.
@saehejkang a couple suggestions so that it fits better with the managed resource plan. I've been swamped with other things and need time to think through the design a bit further for that, but we will get there soon.
| } | ||
|
|
||
| /// List all hostnames in the keychain. | ||
| public func listHosts(id: String) throws -> [String] { |
There was a problem hiding this comment.
Do we need a public version of two calls if we implement the suggestion above?
There was a problem hiding this comment.
I believe we do not
There was a problem hiding this comment.
EDIT: I am actually unsure how this would work. I feel like both functions need to be public, unless I am interpreting your comment incorrectly?
saehejkang
changed the title
jglogan
left a comment
jglogan
left a comment
There was a problem hiding this comment.
@saehejkang Looks good but could you clean up the parameter usage as I mentioned in the comments? Thank you!
| } | ||
| } | ||
|
|
||
| /// List all registry entries for this domain. |
There was a problem hiding this comment.
Could you make the following formal parameter changes in KeychainHelper and KeychainQuery?
- Use
usernameandpasswordfor account name and credentials (notaccountoruser). - Use
securityDomaininstead ofid. - Use
hostnameinstead ofdomainorhost(orregistry domainin docc).
Also can you full docc (description and parameters) on both types?
| public var createdDate: Date | ||
| } | ||
|
|
||
| /// Holds the stored attributes for a registry. |
There was a problem hiding this comment.
Put this in its own file, and add docc for the fields (hostname is a domain name and optional port, for example).
2 participants
Type of Change
Motivation and Context
container registry listcommandRelates to: apple/container#1088
Testing