The ext package
In #43 @inconshreveable wrote:
A separate conversation should probably go into whether the ext package even makes sense and should continue to exist in v3.
Until recently I thought ext was just a folder containing packages for external logging systems, as that is mentioned in #5. I think that does make sense to avoid an eventually huge number of logging-service related packages in the project root.
However, for extra handlers such as the ext.EscalateErrHandler and ext.HotSwapHandler, I think normal sub-packages would be better. And I think extra Handlers are only required to be in a sub-package when one or more of the following is true:
- depends on a third-party package
- compiles with cgo
- imports
unsafe(?) - a large amounts of background work being started on init()
Otherwise, they might just as well be part of log15 itself, right?
As an example; the HotSwapHandler could go into gopkg.in/inconshreveable/log15.v2/hotswap. According to http://blog.golang.org/package-names that could look something like this:
type HotSwapHandler struct {
h log15.Handler
}
func New(h log15.Handler) *HotSwapHandler { … }
func (h *HotSwapHandler) Log(r *log15.Record) error { … }
func (h *HotSwapHandler) Swap(h log15.Handler) { … }
And usage:
import "gopkg.in/inconshreveable/log15.v2/hotswap"
hotswapHandler := hotswap.New(fooHandler)
// etc
// later
hotswapHandler.Swap(barHandler)
So yes, I think ext should continue to exist, but only as folder for logging to external services. As described in #5.
I agree with your criteria for when to include code in the main log15 package.
Perhaps this is a different topic, but 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.
@ChrisHines Excelent point! I hope you don't mind I've opened a new issue for it.
@inconshreveable What do you think about ext ?
I'm not sure about the restriction against unsafe. Log15 already imports unsafe except on appengine.
Yeah me neither.. That's why I had the question mark behind it. Well, now that you say it: appengine could be a valid reason to have unsafe-dependent handlers have their own package. We wouldn't want log15 to become unusable on appengine so it would have to be added with negative build tags. And maybe it makes more sense to say "packages x y and z are not available on appengine" instead of "Handlers x y and z are not available on appengine"? What do you think?
Update on this? Implement it in v3 branch?
We can't remove any existing exported identifiers from packages without an API version bump, so I don't think we want to do this until we have a more compelling reason to create log15.v3.
I've only just skimmed over the bottom of this conversation since I'm tight on time, but, speaking of not wanting to be "unusable on appengine", it seems like we're using "unsafe" and I'm trying to use log15 on AppEngine. Is there a fix for this or am I out of luck with log15?