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
feat: promisify dialog.showCertificateTrustDialog() #17181
Conversation
e0f6179
to
b25f303
Compare
b25f303
to
aff0bc2
Compare
@@ -79,7 +80,8 @@ void ShowCertificateTrust(atom::NativeWindow* parent_window, | |||
CertFreeCertificateChain(chain_context); | |||
} | |||
|
|||
callback.Run(); | |||
promise.Resolve(); |
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.
Is this method is synchronous / blocking on windows? Might be worth documenting / possibly making async somehow 🤔
aff0bc2
to
defd7ae
Compare
7fda78f
to
1e042da
Compare
1e042da
to
092b494
Compare
092b494
to
a80719a
Compare
if ((self = [super init])) { | ||
callback_ = callback; | ||
promise_.reset(new atom::util::Promise(std::move(promise))); |
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.
There is no need keeping promise_
in a std::unique_ptr
. It is only needed when the stored promise may be empty, but in this class since the promise is guaranteed to be always available, it can just be stored as normal value.
@interface TrustDelegate : NSObject {
@private
atom::util::Promise promise_;
}
promise_ = std::move(promise);
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 just realized there is no constructor in objective-c classes so std::unique_ptr
is the only way to go. This PR should be ready to go!
a80719a
to
21a6f45
Compare
21a6f45
to
adfa72a
Compare
Release Notes Persisted
|
* feat: promisify dialog.showCertificateTrustDialog() * update promisification doc
Description of Change
Promisify
dialog.showCertificateTrustDialog()
.Checklist
npm test
passesRelease Notes
Notes: Converted
dialog.showCertificateTrustDialog()
to return a Promise instead of taking a callback.