-
Notifications
You must be signed in to change notification settings - Fork 6
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
API parity with clearTimeout/clearInterval and much housekeeping #9
Conversation
Codecov Report
@@ Coverage Diff @@
## master #9 +/- ##
=======================================
Coverage ? 100%
=======================================
Files ? 1
Lines ? 65
Branches ? 0
=======================================
Hits ? 65
Misses ? 0
Partials ? 0
Continue to review full report at Codecov.
|
@@ -4,64 +4,77 @@ const test = require('tape'); | |||
const timers = require('..'); | |||
|
|||
test('setTimeoutAt', function (t) { | |||
const originalMaxInterval = timers.maxInterval; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about this one. Apparently when I copied the raw file contents from PR #1 (and then added to them), I didn't notice it was using spaces instead of tabs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed last time, but until we add linting rules (I'll do to that later) I wasn't gonna moan about it ;)
}, 10); | ||
}); | ||
|
||
t.test('clearTimeout', function (t) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the actual new changes begin here.
@@ -1,13 +1,17 @@ | |||
{ | |||
"name": "safe-timers", | |||
"version": "1.0.1", | |||
"version": "1.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to bump version numbers in PRs. That's part of the release process.
LGTM. Next time, please refrain from bumping version numbers, as I prefer to do that in the release process. I'll do a quick release after merging this so I'll be sure to remember not to bump the version, but next time we may have a "bump in PR + bump during release" scenario I would like to avoid :) Thanks! |
@ronkorving: Understood. These PRs actually quite go against my personal standard approach to open source contributions (I prefer the 1:1 commit:PR ratio) but I’ve been running into a string of stale open source projects lately whose maintainers rarely if ever do maintenance, so I’ve been providing these “full service PRs” to increase the likelihood of them merging it. |
I also prefer 1:1 commit:PR ratio :) In any case, no problem. And your contributions are much appreciated! |
Welcomed! 💝 |
API parity with
clearTimeout
/clearInterval
(fixes #4) and much housekeeping.Definitely closes #1 completely now, and hopefully is polished off with a new
git tag 1.1.0
andnpm publish
to close #2 completely.