return new entry for Entry.WithContext #941
Conversation
| @@ -103,8 +103,7 @@ func (entry *Entry) WithError(err error) *Entry { | |||
|
|
|||
| // Add a context to the Entry. | |||
| func (entry *Entry) WithContext(ctx context.Context) *Entry { | |||
There was a problem hiding this comment.
Why is the receiver a pointer since you dont manipulate it?
There was a problem hiding this comment.
Keep consistent with Entry.WithError, Entry.WithField and Entry.WithFields methods in the same file.
There was a problem hiding this comment.
That pointer means memory barriers will be different when creating a new Entry. I would argue that the rest of these With* calls should have receiver passed by value.
There was a problem hiding this comment.
That pointer means memory barriers will be different when creating a new Entry. I would argue that the rest of these With* calls should have receiver passed by value.
I think having receiver passed by value or pointer are both ok in this case, just make sure to keep consistent. For the present logic, we create a new Entry inside With* methods and copy some but not all fields from receiver, pointer receiver might be slightly better (less and cleaner code).
…y.WithContext return new entry for Entry.WithContext
Just like Entry.WithError, Entry.WithField and Entry.WithFields, return a new entry for Entry.WithContext. If we don't return a new entry, the old entry with context will be released to entry pool, this may cause some unexpected errors.