-
Notifications
You must be signed in to change notification settings - Fork 876
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
more careful timestamp retrieval #675
Conversation
lib/proto.js
Outdated
@@ -130,7 +130,8 @@ function write (_obj, msg, num) { | |||
// it is not needed anymore | |||
stream.lastMsg = msg | |||
stream.lastObj = obj | |||
stream.lastTime = t.slice(8) | |||
const timeMatch = t.match(/\d+/) |
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.
We shouldn’t have a regular expression in this hot path. It would be better to use .slice
with a correct count.
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.
Added timeSliceIndexSym
with correct count.
test/pretty.test.js
Outdated
@@ -178,7 +178,7 @@ test('with custom timestamp', async ({ is }) => { | |||
cb() |
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’m not sure about overloading this test to test two things. I think there should be separate tests to cover both items.
46555e5
to
e4959cc
Compare
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.
LGTM
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.
LGTM
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Issue #669
It allows to retrieval lastTime value more careful when custom time label is used.