-
Notifications
You must be signed in to change notification settings - Fork 161
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
Update to Developer Beta 3 build. #46
Conversation
The API failure note is irrelevant: we have not shipped the error that is being removed in this patch. |
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.
This looks good from a Swift point of view.
#if (os(macOS) || os(iOS) || os(watchOS) || os(tvOS)) && CRYPTO_IN_SWIFTPM && !CRYPTO_IN_SWIFTPM_FORCE_BUILD_API | ||
@_exported import CryptoKit | ||
#else | ||
import Foundation |
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.
What's being used from Foundation
here?
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.
Nothing, but this is a pretty ordinary starting stanza and I'm loathe to change it. 😉
|
||
let sig = ASN1.ECDSASignature(r: r, s: s) | ||
var serializer = ASN1.Serializer() | ||
try! serializer.serialize(sig) |
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.
Why is the !
okay here?
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.
The !
is ok for two reasons:
- This is a
var
, so we can't throw even if we wanted to: crashing is our only accessible error handling mode - This is not being computed over arbitrary data but previously-validated data, so it should not be possible for this to fail. If it did, it would be our error.
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.
Thanks for the explainer!
No description provided.