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

Allow `#[macro_use(info)]` to import individual macros. #220

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@mjkillough
Copy link

mjkillough commented Aug 22, 2017

Inline logic from log! into error!/warn!/....

Alternatively, we could create a helper function which all macros call (passing in the output of file!/line!/etc.), but this helper function would need to be in the public API of log, which is a bit ugly.

After playing around with alternatives, I figured I may as well open a PR for this option (which I consider to be the least of all evils). If the duplication is undesirable, then feel free to close! :)

Fixes #54.

Allow `#[macro_use(info)]` to import individual macros.
Inline logic from `log!` into `error!`/`warn!`/`...`.

Alternatively, we could create a helper function which all macros call
(passing in the output of `file!`/`line!`/etc.), but this helper
function would need to be in the public API of `log`, which is a bit
ugly.

Fixes #54.
@mjkillough

This comment has been minimized.

Copy link
Author

mjkillough commented Aug 22, 2017

Looking at the proposal in #218, it may be that the macros will soon become more complex, making the code duplication a lot more painful...

@KodrAus

This comment has been minimized.

Copy link
Contributor

KodrAus commented Oct 2, 2017

Thanks for doing this @mjkillough! My personal feeling on this is that we should hold off for now, assuming we want to solve this problem this way, so there's less friction to exploring structured logging.

@sfackler might have other ideas though.

@sfackler

This comment has been minimized.

Copy link
Member

sfackler commented Oct 2, 2017

Yeah, I'd be a bit worried about the implementations desynchronizing, especially as the macros become more complicated. Making a script to generate the module is one option.

@mjkillough

This comment has been minimized.

Copy link
Author

mjkillough commented Oct 2, 2017

Yep, that's fair enough! I'll close this one.

@mjkillough mjkillough closed this Oct 2, 2017

@mjkillough mjkillough deleted the mjkillough:macro_use branch Oct 2, 2017

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.