Skip to content
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

log15.Record compatibility across third-party packages #53

Closed
GeertJohan opened this issue Mar 6, 2015 · 6 comments
Closed

log15.Record compatibility across third-party packages #53

GeertJohan opened this issue Mar 6, 2015 · 6 comments

Comments

@GeertJohan
Copy link

In #51 @ChrisHines wrote:

[..] It is mildly annoying that one must import log15.v2 in order to implement a Handler (because of the *log15.Record argument). So Handlers are always coupled to log15.v2. This may cause problems down the road for people that implement custom handlers in a library. Consumers of such Handlers will have a harder time upgrading to a putative log15.v3 because I'm pretty sure that all the Handlers in an application must have the same import path for *Record. I'm not sure what to do about that yet.

Excelent point! Even though the Record structure stays the same, the types will be incompatible because the qualified identifier is different.

inconshreveable/log15.v2/record
One solution would be moving Record to it's own package log15.v2/record.
Then log15.v3 will still import log15.v2/record, as will everyone that writes a custom handler.
If at v6 we want to make a breaking change the Record type, then log15.v6 will import log15.v6/record, and everyone will have to update their Handlers to work with v6.

inconshreveable/log15record.v1
The strange thing about the above log15.v2/record subpackage is that if you modify the Record in a non-breaking way (add a field at the bottom of the struct), then those changes must be made in log15.v2, even if the most recent log15 version is actually at v4. And it's also a bit strange that log15.v{3,4,5}/record are never used..
So another solution might be to create a new repo inconshreveable/log15record (package record) that starts at v1, and when breaking changes are made moves to v2..

In both cases, Lvl and RecordKeyNames must also move to the new package. Ctx doesn't necessarily have to move because on the record the field type for the context is []interface{}, but if log15.Ctx is used by third-party projects (I don't know if that's the case), it might move to record.Ctx as well to solve the same incompatibility problem.

@inconshreveable
Copy link
Owner

I don't think there's a change here that will make it better, to be honest. It's not ideal that you need to import the package just to implement the Handler interface, but I think any of the other options are just more complicated and burdensome.

@ChrisHines
Copy link
Collaborator

@inconshreveable Agreed.

I have been thinking about this issue, and I'm not convinced there is a good solution either. One approach is to define the Handler interface solely with built in types or types from the standard library. But that may be painful to use.

This issue can serve to document one of the hidden costs of API changes.

@notzippy
Copy link
Contributor

I have had issues with using the versioned form of the URL repo as well. As a solution I have been using a manifest file to pull in the labeled version of the repo directly from github onto my gopath. that works consistent enough for me.

@ChrisHines
Copy link
Collaborator

@notzippy You are talking about gopkg.in/inconshreveable/log15.v2, right? What problems have you had?

@notzippy
Copy link
Contributor

@ChrisHines The code completor for intellij insisted that log15 needed to be on the end of the import statement (like import "gopkg.in/inconshreveable/log15.v2/log15"), this is an issue with the tool more then anything else, but I understand why, it is because the package name in the file is not matching log15.v2 which in theory it should.

@GeertJohan
Copy link
Author

@notzippy I think about 30-40% of all non-std packages use a package name that doesn't match the directory they're in.. Lots of packages are prefixed with go. or go-, and then there's also all the gopkg.in versioned packages with the version suffix. The go tool doesn't expect the directory name to be equal to the package name, not does the spec state that this is expected. So I don't think in theory it should and this is definitely a intellij problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants