Implement porter credential show command #401
Conversation
pkg/porter/credentials.go
Outdated
| return errors.Wrap(err, "unable to determine credentials directory") | ||
| } | ||
|
|
||
| credSet := &credentials.CredentialSet{} |
There was a problem hiding this comment.
Can you break this up into 2 functions: GetCredential and ShowCredential? It makes it easier for testing and automating porter that way.
There was a problem hiding this comment.
Argh, you'd think I would've learned this best practice from the last credentials list PR! Thank you for the reminder (again). Updated.
pkg/porter/credentials.go
Outdated
| // Iterate through all CredentialStrategies to build up our data set | ||
| for _, cs := range credSet.Credentials { | ||
| // Build a reflected Source of type reflectedStruct, for use below | ||
| reflectedSource := reflectedStruct{ |
There was a problem hiding this comment.
I suggest making an exported function that accepts Source and returns the string representing the the source type. It will be easier to read and I bet others will want to use it too. This looks like it was hard to figure out! 馃憤
There was a problem hiding this comment.
Good call; now broken out into exported function.
|
@vdice let's get the changes from @carolynvs in a follow up, i merged this for the goodness |
|
@jeremyrickard @carolynvs I think requested changes were all addressed prior to merge... let me know if not. |
porter credential showcommandSample:
Question(s) for the reviewer:
printerpkg to use the tablewriter enlisted for credential show -- and therefore, apply same table formatting/config for all table printing -- or just keep it for this particular command?reflectcraziness used for determining credential source type a sign I wildly missed an easier route?! 馃槄