Skip to content

timers: remove unused repeat param in timer_wrap #7994

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from

Conversation

jscissr
Copy link
Contributor

@jscissr jscissr commented Aug 6, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)
  • timers
Description of change

The repeat param in start(timeout, repeat) was 0 in all callsites. Because this is an internal API, it can be safely removed.

The `repeat` param in `start(timeout, repeat)` was 0 in all callsites.
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. labels Aug 6, 2016
@Fishrock123
Copy link
Contributor

seems fine to me

@bnoordhuis
Copy link
Member

@mscdex mscdex removed the c++ Issues and PRs that require attention from people who are familiar with C++. label Aug 6, 2016
@mscdex
Copy link
Contributor

mscdex commented Aug 6, 2016

LGTM

@Trott
Copy link
Member

Trott commented Aug 6, 2016

A pair of build failures on CI. Trying again: https://ci.nodejs.org/job/node-test-pull-request/3556/

@cjihrig
Copy link
Contributor

cjihrig commented Aug 8, 2016

LGTM. It looks like there were more build failures.

https://ci.nodejs.org/job/node-test-pull-request/3567/

@jasnell
Copy link
Member

jasnell commented Aug 8, 2016

LGTM

jasnell pushed a commit that referenced this pull request Aug 9, 2016
The `repeat` param in `start(timeout, repeat)` was 0 in all callsites.

PR-URL: #7994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Aug 9, 2016

Landed in 1b99093

@jasnell jasnell closed this Aug 9, 2016
@jscissr jscissr deleted the repeat branch August 9, 2016 19:43
@cjihrig cjihrig mentioned this pull request Aug 10, 2016
cjihrig pushed a commit that referenced this pull request Aug 10, 2016
The `repeat` param in `start(timeout, repeat)` was 0 in all callsites.

PR-URL: #7994
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig mentioned this pull request Aug 11, 2016
@MylesBorins
Copy link
Contributor

adding don't land for now. @Fishrock123 you will likely want to include this if you decide to do the timers backport

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants