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

Restructure initialization setup #230

Merged
merged 2 commits into from Oct 4, 2017

Conversation

Projects
None yet
3 participants
@sfackler
Copy link
Member

sfackler commented Oct 1, 2017

try_set_logger_raw is now set_logger and try_set_logger is now
set_boxed_logger. The use_std feature is now disabled by default. The
old set_logger has been removed.

When we did the crate evaluation for log, we wanted to add more variants
of set_logger that e.g. panicked by default, but I'm no longer convinced
that's a good idea. There are going to be very few instances of actually
calling these methods explicitly, since each logger implementation
should be providing their own init method that calls them. Having a huge
constellation of functions that all do basically the same thing just
makes things really confusing. We also don't want to encourage logger
implementations to only provide an init function that panics because a
common way of working with logging in tests is to try to init the system
in each test and ignore the result.

r? @alexcrichton

/// }
///
/// fn try_init() -> Result<(), SetLoggerError> {
/// log::try_set_logger(|max_log_level| {

This comment has been minimized.

@sfackler

sfackler Oct 1, 2017

Author Member

Both this example and the one below were ignoring the result of try_set_logger....

@sfackler sfackler force-pushed the sfackler:default-no-std branch from f5a4e50 to 057c15c Oct 1, 2017

Restructure initialization setup
try_set_logger_raw is now set_logger and try_set_logger is now
set_boxed_logger. The use_std feature is now disabled by default. The
old set_logger has been removed.

When we did the crate evaluation for log, we wanted to add more variants
of set_logger that e.g. panicked by default, but I'm no longer convinced
that's a good idea. There are going to be very few instances of actually
calling these methods explicitly, since each logger implementation
should be providing their own init method that calls them. Having a huge
constellation of functions that all do basically the same thing just
makes things really confusing. We also don't want to encourage logger
implementations to only provide an init function that panics because a
common way of working with logging in tests is to try to init the system
in each test and ignore the result.

@sfackler sfackler force-pushed the sfackler:default-no-std branch from 057c15c to dc11b85 Oct 1, 2017

@alexcrichton alexcrichton merged commit 4144cf6 into rust-lang-nursery:master Oct 4, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Oct 4, 2017

Sounds great to me!

@meh

This comment has been minimized.

Copy link

meh commented Oct 4, 2017

What's the rationale behind disabling use_std by default?

Took me a while to find I had to enable something to get set_boxed_logger since I just generated the docs and didn't see anything for that.

@sfackler sfackler deleted the sfackler:default-no-std branch Oct 4, 2017

@sfackler

This comment has been minimized.

Copy link
Member Author

sfackler commented Oct 4, 2017

The vast majority of consumers of log don't need to use it, so we're making their lives easier at the expense of the much smaller set of consumers that call set_boxed_logger directly. The docs that end up on docs.rs will include set_boxed_logger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.