util: Fix number format for pad
#21906
Conversation
lib/util.js
Outdated
| @@ -1234,7 +1234,7 @@ function isPrimitive(arg) { | |||
| } | |||
|
|
|||
| function pad(n) { | |||
| return n < 10 ? `0${n.toString(10)}` : n.toString(10); | |||
| return n < 10 ? `0${n}` : n.toString(); | |||
There was a problem hiding this comment.
At this point, we can likely just use padString() ... e.g.
function pad(n) {
return String(n).padStart(2, 0);
}There was a problem hiding this comment.
Fixed it, sorry to forget that!
Many thanks!
|
@lpinca：It seems that the CI is passed but the status is still |
ghost
mentioned this pull request
`pad` is now using `toString(10)`, actually we don't need to do this. As for https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString, `toString(N)` is a radix converter, which isn't proper here for time conversion.
|
@Trott：Maybe this can merged? |
Trott
added
the
author ready
|
Resume Build: https://ci.nodejs.org/job/node-test-pull-request/16362/ |
|
Try freebsd ci again: https://ci.nodejs.org/job/node-test-commit-freebsd/19571/ |
|
Seemingly unrelated failure in freebsd... trying once again https://ci.nodejs.org/job/node-test-commit-freebsd/19575/ |
|
Rebuilding only the failed freebsd: |
|
@BridgeAR：Can this be merged? It seems test-commit always with errors... :( |
|
Rebuilding freebsd again: https://ci.nodejs.org/job/node-test-commit-freebsd/19869/; failure matches #22317 which indicates that #22301 temporarily fixes the problem. |
|
@Trott：Free-bsd passes correctly. |
|
Landed: 6e9e150 Thanks! |
`pad` is now using `toString(10)`, actually we don't need to do this. As for https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString, `toString(N)` is a radix converter, which isn't proper here for time conversion. PR-URL: #21906 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Ruby <rubys@intertwingly.net>
|
Thanks all :D |
`pad` is now using `toString(10)`, actually we don't need to do this. As for https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString, `toString(N)` is a radix converter, which isn't proper here for time conversion. PR-URL: #21906 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Ruby <rubys@intertwingly.net>
`pad` is now using `toString(10)`, actually we don't need to do this. As for https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString, `toString(N)` is a radix converter, which isn't proper here for time conversion. PR-URL: #21906 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Ruby <rubys@intertwingly.net>
padis now usingtoString(10), actually we don't need to do this. As for https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/toString,toString(N)is a radix converter, which isn't proper here for time conversion.PS：Since this is a private function with
na decimal number, so I suspect the code writer wanted to usetoString(10)to convert a number to a two-digit number, and if the number < 10, a0is automatically added before the number itself. Obveriously this isn't a right way. Though the final answer is right.Hope you all take what I mean.
make -j4 test(UNIX), orvcbuild test(Windows) passes