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: clean up test-timers-immediate #8857

Merged
merged 1 commit into from
Oct 3, 2016
Merged

Conversation

Trott
Copy link
Member

@Trott Trott commented Sep 30, 2016

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test tools

Description of change

Clean up test-timers-immediate. Use of let also requires a tweak to
ESLint rules (but it's one that we should do as timers is pretty much
the reason it exists).

@Trott Trott added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Sep 30, 2016
Copy link
Contributor

@Fishrock123 Fishrock123 left a comment

Choose a reason for hiding this comment

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

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

Copy link
Member

@imyller imyller left a comment

Choose a reason for hiding this comment

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

LGTM

@Trott
Copy link
Member Author

Trott commented Sep 30, 2016

Two build failures and one unrelated flakiness on CI.

Not concerned, but going to try again anyway: https://ci.nodejs.org/job/node-test-pull-request/4338/

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

Clean up test-timers-immediate. Use of `let` also requires a tweak to
ESLint rules (but it's one that we should do as timers is pretty much
the reason it exists).

PR-URL: nodejs#8857
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott Trott merged commit b5ec47e into nodejs:master Oct 3, 2016
@Trott
Copy link
Member Author

Trott commented Oct 3, 2016

Landed in b5ec47e

jasnell pushed a commit that referenced this pull request Oct 6, 2016
Clean up test-timers-immediate. Use of `let` also requires a tweak to
ESLint rules (but it's one that we should do as timers is pretty much
the reason it exists).

PR-URL: #8857
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Fishrock123 pushed a commit that referenced this pull request Oct 11, 2016
Clean up test-timers-immediate. Use of `let` also requires a tweak to
ESLint rules (but it's one that we should do as timers is pretty much
the reason it exists).

PR-URL: #8857
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@Trott Trott deleted the test-refactor branch January 13, 2022 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants