feat: add include-timestamp flag #148
Conversation
parse/process.go
Outdated
| } | ||
| fmt.Fprint(option.w, e.Output) | ||
| if e.Output != "" && option.includeTimestamp { | ||
| fmt.Fprint(option.w, e.Time.Format("2006-01-02T15:04:05.000000-07:00")+" "+e.Output) |
There was a problem hiding this comment.
Curious, how did you land on this timestamp format?
There was a problem hiding this comment.
I was implementing the timestamp feature to match the format shown in the JSON output of the issue (2025-05-05T14:14:46.965151-04:00). The goal was to provide consistent timestamp formatting when using the -include-timestamp flag, similar to what's mentioned in the issue.
I went with a custom format string, but I realize now that might not have been the best approach. Would you prefer I use one of the standard time format constants like time.RFC3339Nano, or do you have a different format in mind for this output?
There was a problem hiding this comment.
When I filed the issue I think I just copied that over from GitHub workflow logs.
I bet we can use one of the time package formats. WDYT?
There was a problem hiding this comment.
I've updated the PR to use time.RFC3339Nano instead of the custom format. I believe this should work well for the timestamp output
There was a problem hiding this comment.
I wonder if we need tha much granulrity, do you think time.RFC3339 would be good enough (to reduce the line length).
Last question :)
There was a problem hiding this comment.
that can be a good option , it would reduce the length of the output by 7-8 characters, making it easier to view on a terminal. Updating the PR accordingly.
|
I merged some other PRs in the meanwhile, mind resolving the conflicts? |
resolved |
mfridman
changed the title
2 participants
What
-include-timestampflag to prepend timestamps to follow output lines.e.Time.Format()methodwithout
-include-timestampflag-follow-follow-verbose-follow-outputwith
-include-timestampflag-follow-follow-verbose-follow-outputWhy
closes #141
Test