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

libbindgen: Make logging optional #269

Merged
merged 1 commit into from
Nov 16, 2016

Conversation

jdub
Copy link
Contributor

@jdub jdub commented Nov 15, 2016

Note that the log crate isn't completely banished, as other is required by other dependencies.

Believe it or not, this change was the original aim of #204.

@emilio
Copy link
Contributor

emilio commented Nov 16, 2016

Neat, can we move the stubs to other files? Seems like a lot of burden to be in lib.rs. I'm fine with the rest of the changes (and probably this too if this can't be worked around).

@jdub
Copy link
Contributor Author

jdub commented Nov 16, 2016

Do you prefer:

cfg_if! {
    if #[cfg(feature = "logging")] {
        #[macro_use]
        extern crate log;
    } else {
        #[macro_use]
        mod log_stubs;
    }
}

Or:

#[cfg(feature = "logging")]
#[macro_use]
extern crate log;

#[cfg(not(feature = "logging"))]
#[macro_use]
mod log_stubs;

@emilio
Copy link
Contributor

emilio commented Nov 16, 2016

The second looks nicer IMO, you can drop that comment and we'll land this :)

Note that the log crate isn't completely banished, as other is required
by other dependencies.
@jdub jdub force-pushed the logless-library-really branch from 19c6dd2 to a949b5c Compare November 16, 2016 11:22
@jdub
Copy link
Contributor Author

jdub commented Nov 16, 2016

Ready to go.

@emilio
Copy link
Contributor

emilio commented Nov 16, 2016

@bors-servo r+

@bors-servo
Copy link

📌 Commit a949b5c has been approved by emilio

@bors-servo
Copy link

⌛ Testing commit a949b5c with merge c800ad4...

bors-servo pushed a commit that referenced this pull request Nov 16, 2016
libbindgen: Make logging optional

Note that the log crate isn't completely banished, as other is required by other dependencies.

Believe it or not, this change was the original aim of #204.
@bors-servo
Copy link

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit a949b5c into rust-lang:master Nov 16, 2016
@jdub jdub deleted the logless-library-really branch November 16, 2016 11:46
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

Successfully merging this pull request may close these issues.

4 participants