Rewrite liblog into a logging facade

[sfackler]

See issue #3 for background.

Closes #3
Closes #7
Closes #11

[rust-highfive]

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

[sfackler]

I have a port of the old logging implementation here: https://github.com/sfackler/log-ng/tree/master/basic

Do you think it makes sense to keep it in this repo or in another?

[alexcrichton]

Does derive not work here?

[sfackler]

derive generates really bad code for all of these methods.

[alexcrichton]

Ah ok!

[alexcrichton]

Do you think it makes sense to keep it in this repo or in another?

Yeah let's keep at least one implementation here in the repo. There should probably be an example or two as well showing how to depend on it and use it.

[alexcrichton]

I chatted with some others about rewriting some of the libraries in rust-lang, and our general conclusion was that if a total rewrite is happening it should probably happen out of repo, but if an API overhaul is happening (e.g. changing to a builder interface for getopts from free functions) then it's fine to stick to the same repo for now (with a version bump).

I think that this falls into the category of API overhaul rather than total rewrite (despite it being a total rewrite!) due to the macros staying the same. If the old behavior is kept in this repo as a "basic" crate then I'm totally fine keeping it all in this repo.

Thanks so much for working on this @sfackler, this all looks pretty awesome to me and I'm excited to see where it goes!

[sfackler]

Ok, pushed a bunch of changes for your comments, and added the 1.0 logger port as a separate crate. It's named env_logger right now but I'm not a huge fan of the name. It does need some module level documentation - I'll add that and an expanded README tomorrow.

One thing I have been thinking about is the possibility of reexporting the logging macros from env_logger so it could be used without requiring a separate log import. It'd have to reexport LogLocation, LogRecord, LogLevel, max_log_level, enabled, and log, but we could hide those behind #[doc(hidden)]. Thoughts?

[sfackler]

It'd be the difference between

[dependencies]
log = "0.2"
env_logger = "0.1"

#[macro_use]
extern crate log;
extern crate env_logger;

and

[dependencies]
env_logger = "0.1"

#[macro_use]
extern crate env_logger;

Pretty minimal, but mildly more convenient for quick things.

[alexcrichton]

I'll add that and an expanded README tomorrow.

Thanks! Here's a list of what I can think is left

• README explaining how to use liblog in a library we well as in a binary (using env_logger as the example).
• Docs for env_logger basically just saying to go look at liblog as well as how you should only use it to call init
• Travis updates to run tests for env_logger (I can do this)
• Travis publishes docs for env_logger as well as logger (I can do this)

And... I think that's it! All the other docs look good to me.

One thing I have been thinking about is the possibility of reexporting the logging macros from env_logger so it could be used without requiring a separate log import.

I'd probably prefer to keep the two separate for now as it may be annoying to "require" all logging crates to reexport the logging macros. If it gets too painful in the future though we an definitely look into reexporting.

[sfackler]

SGTM

[sfackler]

Docs and README have been updated.

[sfackler]

One last thing to note is that LogLocation's fields are still public. They have to be that way so that they can be stored in statics next to the log calls, but we could set all of the fields to #[doc(hidden)] and say that the contents don't have any stability guarantees.

[seanmonstar]

This somewhat silently turns all debug!() calls on in release builds, unless a cfg is passed?

[sfackler]

Yep. There's some discussion of it in #3. The higher verbosity logging levels are pretty important to keep around in release builds in my experience, because they're often one of the only options you have if an issue pops up that's hard/infeasible to reproduce outside of a production instance. The cfgs offer a more uniform interface to completely disabling logging at whatever level you'd like.

[seanmonstar]

Yea I saw that. I guess as long as the expensive part is inside the fmt
function, it's not being called unless you want to see it, so that's fine
with me.

On Tue, Jan 27, 2015, 10:07 AM Steven Fackler [email protected]
wrote:

Yep. There's some discussion of it in #3
#3. The higher verbosity logging
levels are pretty important to keep around in release builds in my
experience, because they're often one of the only options you have if an
issue pops up that's hard/infeasible to reproduce outside of a production
instance. The cfgs offer a more uniform interface to completely disabling
logging at whatever level you'd like.

—
Reply to this email directly or view it on GitHub
#12 (comment).