doc: remove reference to "credentials object" by sam-github · Pull Request #26908 · nodejs/node (original) (raw)

Conversation

This file contains hidden or 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

[ Show hidden characters]({{ revealButtonHref }})

The reference is confusing because the object is actually of class
SecureContext. There is no object with class "credentials".

See: #20432 (comment)

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot added doc

Issues and PRs related to the documentations.

tls

Issues and PRs related to the tls subsystem.

labels

Mar 25, 2019

The reference is confusing because the object is actually of class SecureContext. There is no object with class "credentials".

See: nodejs#20432 (comment)

@@ -1406,7 +1406,9 @@ to `true`, other APIs that create secure contexts leave it unset.
from `process.argv` as the default value of the `sessionIdContext` option, other
APIs that create secure contexts have no default value.

The `tls.createSecureContext()` method creates a credentials object.
The `tls.createSecureContext()` method creates a `SecureContext` object. The
object has no public methods, but is accepted as an argument to several `tls`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Total micro-nit that you can ignore if you disagree, but I think the two sentences are clearer if you divide them up this way instead:

The tls.createSecureContext() method creates a SecureContext object with no public methods. It is usable as an argument to several tls APIs such as [tls.createServer()][] and [server.addContext()][].

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point, how about ba029f5?

BridgeAR added the author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

label

Mar 26, 2019

sam-github added a commit that referenced this pull request

Mar 28, 2019

The reference is confusing because the object is actually of class SecureContext. There is no object with class "credentials".

See: #20432 (comment)

PR-URL: #26908 Reviewed-By: Colin Ihrig cjihrig@gmail.com Reviewed-By: Luigi Pinca luigipinca@gmail.com Reviewed-By: Ruben Bridgewater ruben@bridgewater.de Reviewed-By: Ben Noordhuis info@bnoordhuis.nl

targos pushed a commit that referenced this pull request

Mar 28, 2019

The reference is confusing because the object is actually of class SecureContext. There is no object with class "credentials".

See: #20432 (comment)

targos pushed a commit that referenced this pull request

Mar 29, 2019

The reference is confusing because the object is actually of class SecureContext. There is no object with class "credentials".

See: #20432 (comment)

targos pushed a commit that referenced this pull request

Mar 30, 2019

The reference is confusing because the object is actually of class SecureContext. There is no object with class "credentials".

See: #20432 (comment)

@sam-github Why does SecureContext not have any public methods?

Specifically, secureContext.context.addCACert(...) seems to be available and quite helpful. See #20432 (comment) and #20432 (comment) for more details.

@cinderblock I can't speak to the history, but I can say if there is a specific feature you would like, please post a feature request. It will help to have some concrete use cases. There are lots of things we could just make public, but it takes short term effort to add tests and docs (though feature requests with an acompanying PR don't have to attract a volunteer), and long-term, it can make it harder to improve the node API if too much of our internals are exposed in the API.

BethGriggs pushed a commit that referenced this pull request

Apr 5, 2019