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

test: update var to const, use arrow functions #8595

Conversation

matt-in-a-hat
Copy link
Contributor

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
Affected core subsystem(s)

test

Description of change
  • Replaced var with const.
  • Replaced anonymous function with arrow function.

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Sep 17, 2016
@mscdex mscdex added the cluster Issues and PRs related to the cluster subsystem. label Sep 17, 2016
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

LGTM with a green CI

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM

@jasnell
Copy link
Member

jasnell commented Sep 22, 2016

@addaleax
Copy link
Member

@matt-in-a-hat You author name in this commit is given as “matt-in-a-hat”. Is that intended or do you prefer to be listed (changelog, git log, AUTHORS file) with some other name? People typically prefer their full name, but ultimately it’s up to you.

@matt-in-a-hat
Copy link
Contributor Author

I'm not too worried, if to change it I have to force push the commit and re-do CI etc then don't worry about it.

jasnell pushed a commit that referenced this pull request Sep 27, 2016
PR-URL: #8595
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 27, 2016

Landed in 7561123. I took the liberty of updating the name in the commit when landing.

@jasnell jasnell closed this Sep 27, 2016
jasnell pushed a commit that referenced this pull request Sep 29, 2016
PR-URL: #8595
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
PR-URL: #8595
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants