Skip to content
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

doc: fix echo example programs #24235

Closed
wants to merge 1 commit into from

Conversation

sam-github
Copy link
Contributor

Adjust to work with self-signed certificates, and certificates that do
not name "localhost" as their host name.
Removed duplicate examples, they differed only by using pfx. Its not
necessary to show every option, and we don't, and the example wouldn't
work with most pfx anyway, since it didn't specify a password.

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 nodejs-github-bot added doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem. labels Nov 7, 2018
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

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

Just some nits.

doc/api/tls.md Outdated Show resolved Hide resolved
doc/api/tls.md Outdated Show resolved Hide resolved
doc/api/tls.md Outdated Show resolved Hide resolved
Adjust to work with self-signed certificates, and certificates that do
not name "localhost" as their host name.
Removed duplicate examples, they differed only by using `pfx`. Its not
necessary to show every option, and we don't, and the example wouldn't
work with most pfx anyway, since it didn't specify a password.
@sam-github
Copy link
Contributor Author

@vsemozhetbyt PTAL

@vsemozhetbyt
Copy link
Contributor

The format of changes LGTM, but I am not competent enough in TLS to evaluate all the reductions)

@sam-github
Copy link
Contributor Author

@nodejs/crypto PTAL

@sam-github
Copy link
Contributor Author

@sam-github
Copy link
Contributor Author

You don't have to be a TLS expert to read two long example programs and realize they are identical, except for 2 lines! ;-)

@nodejs/crypto PTAL

@danbev
Copy link
Contributor

danbev commented Nov 13, 2018

Landed in 05a11d1.

@danbev danbev closed this Nov 13, 2018
danbev pushed a commit that referenced this pull request Nov 13, 2018
Adjust to work with self-signed certificates, and certificates that do
not name "localhost" as their host name.
Removed duplicate examples, they differed only by using `pfx`. Its not
necessary to show every option, and we don't, and the example wouldn't
work with most pfx anyway, since it didn't specify a password.

PR-URL: #24235
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
@sam-github sam-github deleted the fix-echo-examples branch November 13, 2018 22:27
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Adjust to work with self-signed certificates, and certificates that do
not name "localhost" as their host name.
Removed duplicate examples, they differed only by using `pfx`. Its not
necessary to show every option, and we don't, and the example wouldn't
work with most pfx anyway, since it didn't specify a password.

PR-URL: #24235
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
Adjust to work with self-signed certificates, and certificates that do
not name "localhost" as their host name.
Removed duplicate examples, they differed only by using `pfx`. Its not
necessary to show every option, and we don't, and the example wouldn't
work with most pfx anyway, since it didn't specify a password.

PR-URL: nodejs#24235
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
codebytere pushed a commit that referenced this pull request Jan 12, 2019
Adjust to work with self-signed certificates, and certificates that do
not name "localhost" as their host name.
Removed duplicate examples, they differed only by using `pfx`. Its not
necessary to show every option, and we don't, and the example wouldn't
work with most pfx anyway, since it didn't specify a password.

PR-URL: #24235
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: Daniel Bevenius <[email protected]>
@codebytere codebytere mentioned this pull request Jan 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants