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.

Sign up for GitHub

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

Jump to bottom

test: timers.js fails lint check in master #5823

Closed
mhdawson opened this issue Mar 21, 2016 · 4 comments
Closed

test: timers.js fails lint check in master #5823

mhdawson opened this issue Mar 21, 2016 · 4 comments
Labels
test Issues and PRs related to the tests. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Comments

@mhdawson
Copy link
Member

  • Version: master
  • Platform: N/A
  • Subsystem: timers:

Started run of jslint on master after seeing error in checking out a PR that did not chagne timers.js at all

https://ci.nodejs.org/job/node-test-linter/1744/

Fails with

+ gmake lint
./node tools/eslint/bin/eslint.js benchmark lib src test tools/doc \
    tools/eslint-rules --rulesdir tools/eslint-rules

/usr/home/iojs/build/workspace/node-test-linter/lib/timers.js
  450:2  error  Unnecessary semicolon  no-extra-semi
@MylesBorins MylesBorins added timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. meta Issues and PRs related to the general management of the project. labels Mar 21, 2016
@MylesBorins
Copy link
Contributor

@mhdawson this is definitely broken 😢

Looks like it happened in this commit 4fe02e2. Switching from a function literal to a function expression removed the need of a semi colon. This specific commit was not put through CI, and thus the lint issue was not found.

/cc @Fishrock123

@mhdawson mhdawson added test Issues and PRs related to the tests. and removed meta Issues and PRs related to the general management of the project. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. labels Mar 21, 2016
@mhdawson
Copy link
Member Author

Will put together a PR to vix

@mscdex mscdex added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Mar 21, 2016
@Fishrock123
Copy link
Contributor

doh

@Fishrock123 Fishrock123 mentioned this issue Mar 21, 2016
Merged
2 tasks
Fishrock123 added a commit to Fishrock123/node that referenced this issue Mar 21, 2016
@Fishrock123
timers: fix lint from 4fe02e2
d3a7534 
Fixes: nodejs#5823
Refs: nodejs#5793
PR-URL: nodejs#5825
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@Fishrock123
Copy link
Contributor

my bad, fixed in d3a7534

@joaocgreis joaocgreis mentioned this issue Mar 21, 2016
Closed
3 tasks
Fishrock123 added a commit that referenced this issue Mar 22, 2016
@Fishrock123
timers: fix lint from 4fe02e2
46f0e02 
Fixes: #5823
Refs: #5793
PR-URL: #5825
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test Issues and PRs related to the tests. timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mscdex @MylesBorins @Fishrock123 @mhdawson