Skip to content

Conversation

@DaftMonk
Copy link
Contributor

Document current behavior of TLSSocket, as discussed in #3545

@r-52r-52 added tls Issues and PRs related to the tls subsystem. doc Issues and PRs related to the documentations. labels Nov 10, 2015
@mscdex
Copy link
Contributor

When I first read this, I didn't know what methods were being referred to. You might include an example method (e.g. getPeerCertificate()) or two.

@DaftMonk
Copy link
ContributorAuthor

@mscdex Added an example.

Copy link
Member

Choose a reason for hiding this comment

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

Please wrap at 80 columns. Maybe you can turn getPeerCertificate() into a link?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Updated to use correct column length.

I couldn't get intra-page links working for that section. I tried using the markdown

[getPeerCertificate](#tlssocketgetpeercertificate-detailed-) 

and when I opened the preview in github and clicked on it, it didn't work. However typing in the url directly https://github.com/DaftMonk/node/blob/docs-tls-socket/doc/api/tls.markdown#tlssocketgetpeercertificate-detailed- did work.

So I'm not sure what the issue was, but I reverted it back to a code example.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Never mind, I tried again and the link worked this time.

@DaftMonkDaftMonkforce-pushed the docs-tls-socket branch 5 times, most recently from 1f04c0b to 0a414e8CompareNovember 10, 2015 21:24
@bnoordhuis
Copy link
Member

Thanks, landed in eff8c3e.

bnoordhuis pushed a commit that referenced this pull request Nov 10, 2015
Fishrock123 pushed a commit that referenced this pull request Nov 11, 2015
@Fishrock123Fishrock123 mentioned this pull request Nov 11, 2015
MylesBorins pushed a commit that referenced this pull request Nov 17, 2015
@MylesBorins
Copy link
Contributor

landed in v4.x-staging in 8b6120d

rvagg pushed a commit that referenced this pull request Dec 4, 2015
@jasnelljasnell mentioned this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 17, 2015
jasnell pushed a commit that referenced this pull request Dec 23, 2015
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@DaftMonk@mscdex@bnoordhuis@MylesBorins@r-52