-
Notifications
You must be signed in to change notification settings - Fork 75
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
(feature) Return CA Certificate path/data #41
Merged
Merged
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
3032280
Fixes typescript runtime errors
9ce6762
Encryption/chmod not required for certificate, only key
44ca076
Add run options & branch that returns caPath/caCert Buffer
ecc8e18
Reverse formatting changes
3443dc5
separate API concerns
Js-Brecht b7c3857
Add support for both `getCaPath` and `getCaBuffer` API interfaces
Js-Brecht a8948d9
TSDoc update
Js-Brecht 609a693
Update conditions / return type
Js-Brecht e87fb6d
Add additional flags to main function TSDoc
Js-Brecht d561a07
Merge branch 'master' into return-ca-cert
zetlen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@zetlen Because of the additional flexibility of the API, I wrote these types that will cause the return value to be more explicit, depending on what flags you set when you call
certificateFor()
.Let me know if you think this is too much/too complicated. The alternative is to limit it to two different return types, but then you wind up with
Partial
properties:This probably wouldn't be an issue, ultimately. I just wanted it to be more clear what you're getting for what you give, and also get the benefits of strong types.
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 self-documenting types!
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 approve of the types; there are lot of ways to type this, but if it populates autosuggest correctly, then that's all that matters. Thanks for thinking about it carefully.