-
Notifications
You must be signed in to change notification settings - Fork 510
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
Notary client interface type #1220
Notary client interface type #1220
Conversation
Signed-off-by: David Lawrence <[email protected]> (github: endophage)
… arg Signed-off-by: Riyaz Faizullabhoy <[email protected]>
Thanks for wrapping up the last bits of cleanup on this. Will review once CI passes. |
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.
LGTM
return filepath.Walk(sourceDir, func(fp string, fi os.FileInfo, err error) error { | ||
if err != nil { | ||
return err | ||
} | ||
|
||
targetFP := filepath.Join(targetDir, strings.TrimPrefix(fp, sourceDir+"/")) |
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.
Happy to see we finally get rid of the +"/"
Haven't check it line by line, but I like the design |
client/interface.go
Outdated
// Key Operations | ||
RotateKey(role data.RoleName, serverManagesKey bool, keyList []string) error | ||
|
||
CryptoService() signed.CryptoService |
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.
s/CryptoService/GetCryptoService/g
LGTM after nit on GetCryptoService(). |
Signed-off-by: Riyaz Faizullabhoy <[email protected]>
b28e526
to
62788b4
Compare
Repository
interface to standardizeclient.NotaryRepository
InitializeWithCertificate
to not take a notary repository as an argumentPart of #1139, originally stolen from
endophage/client_interface
branch