-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Apply time prefix to plain (terminal) logs #3212
Conversation
2a7e11d
to
d5c5c69
Compare
detox/src/utils/logger.js
Outdated
@@ -11,6 +14,9 @@ const temporaryPath = require('../artifacts/utils/temporaryPath'); | |||
const argparse = require('./argparse'); | |||
const customConsoleLogger = require('./customConsoleLogger'); | |||
|
|||
const LOG_DATE_FORMAT = "HH:MM:ss.l"; | |||
const shortDateFormatter = (date) => dateFormat(date, LOG_DATE_FORMAT); |
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.
Won't it be more efficient to:
date.toTimeString().split(' ')[0] + '.' + `${date.getMilliseconds()}`.padStart(3, '0')
instead of importing yet another package?
+"dateformat": "4.5.1",
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.
Definitely. I will do that 👍🏻
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 was contemplating whether ןא would be better to have this for future usage. On 2nd thought, I'm inclined to say that since this hasn't troubled us this long, the odds of it being so are slim.
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 see what you mean. If we ever need more advanced date manipulation, we can add it later, no problem.
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.
@noomorph I've taken a rather more verbose approach, that is nevertheless aligned with your suggestion. Please have a glance.
80f0a97
to
30550a7
Compare
Description
In this pull request, I have enabled a custom (short) timestamp prefix to terminal logs. Other log outputs should have remained intact.
Before:
After: