-
Notifications
You must be signed in to change notification settings - Fork 9
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
changed position of level to be after timestamp #31
base: master
Are you sure you want to change the base?
Conversation
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.
One question about the change in the formatter
message := strings.Builder{} | ||
message.WriteString(entry.Time.Format(time.RFC3339Nano)) | ||
message.WriteByte(' ') | ||
sections := append(make([]string, 0, 5), entry.Time.Format(time.RFC3339Nano), strings.ToUpper(entry.Level.String())) |
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.
Interesting idea, does this provide a performance improvement?
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.
it's really just because I was frustrated with managing the whitespaces though I remembered I tried comparing Join
and using the Builder
(my test was small) and there wasn't really any difference, at least with small cases.
Please document the why in the description (and make sure it ends up in the commit message). |
@anzdaddy description updated |
message.WriteByte('\n') | ||
return []byte(message.String()), nil | ||
sections = append(sections, "\n") | ||
return []byte(strings.Join(sections, " ")), nil |
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.
This will incur a performance hit due to copying the final string into a byte array. Use bytes.Buffer
instead. Maybe also NewBytesBuffer(make([]byte, 0, aReasonableDefaultCapacity))
.
* move to prod runner and fix tag action
Standard format is now changed to
Also removed some dead code
The reason for the change is that, previously
level
is put afterfields
and it is difficult to find the level as the number of fields varies. Putting it next totimestamps
just makes it easier to find.