Skip to content

Update tls_wrap.cc - #2486

Closed
JungMinu wants to merge 3 commits into
nodejs:masterfrom
JungMinu:patch-1
Closed

Update tls_wrap.cc#2486
JungMinu wants to merge 3 commits into
nodejs:masterfrom
JungMinu:patch-1

Conversation

@JungMinu

Copy link
Copy Markdown
Member

fix grammatically wrong expression

fix grammatically wrong expression
@mscdexmscdex added tls Issues and PRs related to the tls subsystem. c++ Issues and PRs that require attention from people who are familiar with C++. labels Aug 21, 2015
@silverwindsilverwind added doc Issues and PRs related to the documentations. and removed c++ Issues and PRs that require attention from people who are familiar with C++. labels Aug 21, 2015
Comment threadsrc/tls_wrap.cc Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is an improvement but there are still two issues in the comment here (ex 'socket' -> 'the socket', 'callback' -> 'the callback'), would you mind fixing the rest while you're here? :)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@brendanashworth Thanks! I fixed the rest :)

fix grammatically wrong expression
@JungMinu

Copy link
Copy Markdown
MemberAuthor

@brendanashworth Thanks for the comment!
I fixed the rest!

@jasnell

Copy link
Copy Markdown
Member

LGTM

@JungMinu

Copy link
Copy Markdown
MemberAuthor

@jasnell Thanks!

@jasnell

Copy link
Copy Markdown
Member

Can I ask you to please squash the commits down into a single and update the commit log to the pattern shown here: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md#step-3-commit. Once that's done I'll queue it up to land.

@JungMinuJungMinu reopened this Aug 22, 2015
@JungMinu

Copy link
Copy Markdown
MemberAuthor

@jasnell I remade a new pull request into a single commit and updated the commit log:
#2490

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.

6 participants

@JungMinu@jasnell@brendanashworth@mscdex@silverwind@Fishrock123