locking: reintroduce Committer field#1863
Conversation
| func newCommitter(name, email string) committer { | ||
| return committer{Name: name, Email: email} | ||
| func NewCommitter(name, email string) Committer { | ||
| return Committer{Name: name, Email: email} |
There was a problem hiding this comment.
Should this be returning a *Committer, since that's the receiver for String() below? Or should the func change the receiver to the struct value?
There was a problem hiding this comment.
Good question. The constructor function returns a raw-value Committer, since that's how it's embedded in the Lock type. Lock prefers a Committer over a *Committer, since the extra data needed to store the pointer of an immutable value is unnecessary.
However, there's no danger of mutable behavior happening in the (*Committer) String() string method, so copying the value is unnecessary. Go will automatically take the address to call methods defined on the pointer type, i.e. in a type defined like:
type T struct {}
func (t *T) Foo() {}calling
t := T{}
t.Foo()is equivalent to calling:
t := T{}
(&t).Foo()
This pull-request reintroduces the
Committerfield on the*locking.Locktype.This was the original interface of that type, but was removed in #1732 in favor of
NameandEmailtop-level fields on theLocktype. Instead, let's bring back the original API which:This pull-request also comes with some additional benefits:
gitserverwas unable to find the fields to de-serialize the committer's name and email, thus making all locks have""empty-string owners.fmt.Stringeron theCommittertype, which yields the expected format:First Last <email@example.com>./cc @git-lfs/core