-
Notifications
You must be signed in to change notification settings - Fork 52
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
Implement sign
command
#21
Conversation
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.
I think this is looking good. Are you planning to add anything to this?
SCT verification is not yet complete
@tetsuo-cpp I'll let you decide if we should lower or disable the coverage bar here, or try and add some very early tests. I think the former is fine as long as we come back later and test what we've got. |
pass | ||
|
||
|
||
def sign(file_, identity_token, output=_no_output): |
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.
I think we should figure out what the return type should be soon. I'm thinking something like a SigningResult
which allows you to tell between success/failure and whether there is a reason.
We could also just not have a return and consider any non-throwing call to sign
to be a success.
I'll go ahead and merge this anyway since we have too many pieces of work branched off this, but we should definitely revisit it.
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.
Agreed, although I don't think an empty return would work for anyone using this as an importable API, they'd want roughly the same things we're outputting (the signature and certificate) returned. I like the idea of a SigningResult
.
Fixes #9.