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, remove unnecessary bind(this) #4797

Closed
wants to merge 1 commit into from
Closed

Conversation

wKich
Copy link
Contributor

@wKich wKich commented Jan 21, 2016

No description provided.

@benjamingr
Copy link
Member

Might as well make it an expression (single-line) arrow function

@benjamingr
Copy link
Member

LGTM.

@wKich
Copy link
Contributor Author

wKich commented Jan 21, 2016

I think will need to refactor all other arrow functions, If make it as a single-line expression, for uniform code style.

@mscdex mscdex added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Jan 21, 2016
@Fishrock123
Copy link
Contributor

I think usually keeping the braces will help understandability in docs for newcomers.

cc @nodejs/documentation

@evanlucas
Copy link
Contributor

+1 to keeping the braces

@cjihrig
Copy link
Contributor

cjihrig commented Jan 21, 2016

Also +1 to keeping braces.

LGTM as is.

@chrisdickinson
Copy link
Contributor

LGTM. The brace/no-brace disagreement emphasizes the degree to which we need to put a linter in the doctool.

@benjamingr
Copy link
Member

Related nodejs/docs#62

silverwind pushed a commit that referenced this pull request Jan 26, 2016
PR-URL: #4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
@silverwind
Copy link
Contributor

Thanks! Landed in 2c42635.

@silverwind silverwind closed this Jan 26, 2016
rvagg pushed a commit that referenced this pull request Jan 27, 2016
PR-URL: #4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
benjamingr pushed a commit to benjamingr/io.js that referenced this pull request Jan 27, 2016
PR-URL: nodejs#4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
rvagg pushed a commit that referenced this pull request Feb 8, 2016
PR-URL: #4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
MylesBorins pushed a commit that referenced this pull request Feb 17, 2016
PR-URL: #4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
PR-URL: #4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
@MylesBorins MylesBorins mentioned this pull request Feb 18, 2016
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
PR-URL: #4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
PR-URL: nodejs#4797
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chris Dickinson <chris@neversaw.us>
Reviewed-By: Roman Reiss <me@silverwind.io>
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. process Issues and PRs related to the process subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants