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

Update tls_wrap.cc #2486

Closed
wants to merge 3 commits into from
Closed

Update tls_wrap.cc #2486

wants to merge 3 commits into from

Conversation

JungMinu
Copy link
Member

fix grammatically wrong expression

fix grammatically wrong expression
@mscdex mscdex 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
@silverwind silverwind 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
@@ -561,7 +561,7 @@ int TLSWrap::DoWrite(WriteWrap* w,
}
if (empty) {
ClearOut();
// However if there any data that should be written to socket,
// However if there is any data that should be written to socket,
Copy link
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
Member Author

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
Member Author

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

@jasnell
Copy link
Member

jasnell commented Aug 22, 2015

LGTM

@JungMinu
Copy link
Member Author

@jasnell Thanks!

@jasnell
Copy link
Member

jasnell commented Aug 22, 2015

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.

@JungMinu JungMinu closed this Aug 22, 2015
@JungMinu JungMinu reopened this Aug 22, 2015
@JungMinu
Copy link
Member Author

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

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