simplify public interface - replace *Entry with Interface in interface.go

[client9]

Hello!

In the spirit of simplifying the logrus API, this PR replaces *Entry in the public interface to be Interface. Most of the changes were mechanical in nature (it just worked). One private helper function was needed in Entry (near trivial). Some casts were needed in the tests.

Totally understand if you don't merge,since this is a semver API change. But if you are interested in a v2, I think interface.go can become completely self-contained. This would step 1 to do so.

Thanks for an interesting project!

n

 type Interface interface {		 
	WithFields(fields Fielder) *Entry
	WithField(key string, value interface{}) *Entry
	WithError(err error) *Entry
(and others)

After:

type Interface interface {
        WithFields(fields Fielder) Interface
        WithField(key string, value interface{}) Interface
        WithError(err error) Interface
// (and others)
}

[client9]

failed to do golang stuff unrelated to the PR

# github.com/apex/log/handlers/delta
handlers/delta/delta.go:140: time.Since(h.start).Round undefined (type time.Duration has no field or method Round)
handlers/delta/delta.go:142: time.Since(h.start).Round undefined (type time.Duration has no field or method Round)

[tj]

ahhh sweet I'll see if Semaphore has a more recent Go version now, and I'm down with a semver bump, I'm all for cleaning that up :D I'll take a closer look soon

[client9]

rocking.

It's likely you'll want to do something a bit different, especially in tests with the casting.

If v2 is on the table then, in my fantasy world, the Fielder interface would be replaced by just Fields (back to logrus), and the Fielder methods would be replaced by functions (get the names, sort, etc) to keep the interface small. But I'm sure you have reasons to use the interface.

Otherwise, If we moved the following from logger.go into interface.go then interface.go is completely self-contained. A dev can look at it and know everything on how to log (handlers and configuration being a different matter).

onward!

// Fielder is an interface for providing fields to custom types.
type Fielder interface {
	Fields() Fields
}

// Fields represents a map of entry level data used for structured logging.
type Fields map[string]interface{}

[tj]

Otherwise, If we moved the following from logger.go into interface.go then interface.go is completely self-contained. A dev can look at it and know everything on how to log (handlers and configuration being a different matter).

SGTM!

I still use the Fielder() interface quite a bit, but it is a little ugly to tack anything logging related onto structs hahah so I don't like that aspect of it.

[tj]

Looks good to me! Just slap that one comment in there and it should be good to go

[tj]

Don't forget a comment :DDD

[client9]

ha will do (comment and move interface definitions).

[client9]

ah crap I commited the wrong file... one sec.

[client9]

ahh found where you use Fielder

// WithError returns a new entry with the "error" set to err.
//
// The given error may implement .Fielder, if it does the method
// will add all its .Fields() into the returned entry.

[tj]

Updated Semaphore to use 1.9, looks like it's failing on:

./logger_test.go:96:57: l.WithField("file", "sloth.png").Trace("upload").Stop undefined (type log.Interface has no field or method Stop)
./logger_test.go:127:57: l.WithField("file", "sloth.png").Trace("upload").Stop undefined (type log.Interface has no field or method Stop)
./logger_test.go:159:57: l.WithField("file", "sloth.png").Trace("upload").Stop undefined (type log.Interface has no field or method Stop)
./pkg_test.go:74:27: log.Trace("upload").Stop undefined (type log.Interface has no field or method Stop)

[client9]

Oh I see.. needs some casts back to Entry for tests. Not changes needed to regular code.

weird I didn't see this before.

on it.

[client9]

Oh I see the problem... never sure how to solve this in golang + github.

My branch uses:

import(
...
        "github.com/apex/log"

instead of my fork, so I don't see the error locally.

[client9]

@tj ready for re-review thx.

[timbunce]

Hi. Anything blocking this?

[tj]

@timbunce it's a bit of a breaking change so it'll have to be in 2.0, and I'm hoping to make some other changes in 2.0 as well but hard to say when that'll be

[timbunce]

Thanks for the update @tj. I'm glad to know you're planning for a v2. Can those of us interested in the module help out in any way?

[tj]

@timbunce not at the moment, I'll try and figure out what I'd like to do for the next version and start a branch that people could help out on, thanks for offering to help :D