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

Enable use of Logger as a filter #4

Merged
merged 2 commits into from Sep 15, 2017

Conversation

Projects
None yet
3 participants
@jethrogb
Copy link
Contributor

jethrogb commented Jul 12, 2017

The spec parsing and matching of env_logger is very useful, but it will only let you log to stdout/stderr. With this change you can embed env_logger's logic in your own logger.

@sebasmagri

This comment has been minimized.

Copy link
Owner

sebasmagri commented Jul 12, 2017

@jethrogb it looks good to me initially. However, could you please add an example or a few documentation lines explaining how to use this with your own logger?

@sebasmagri sebasmagri requested a review from sfackler Jul 12, 2017

@jethrogb

This comment has been minimized.

Copy link
Contributor Author

jethrogb commented Jul 12, 2017

Added docs

@jethrogb jethrogb force-pushed the jethrogb:filter branch from 3a5b1f9 to 1e87fdc Jul 12, 2017

@sebasmagri

This comment has been minimized.

Copy link
Owner

sebasmagri commented Aug 22, 2017

Hi @jethrogb!

Could you please review and rebase changes against the latest master to merge this?

@jethrogb jethrogb force-pushed the jethrogb:filter branch from 1e87fdc to 1eaa005 Aug 22, 2017

@jethrogb

This comment has been minimized.

Copy link
Contributor Author

jethrogb commented Aug 22, 2017

Rebased

@jethrogb jethrogb force-pushed the jethrogb:filter branch from 1eaa005 to d7e6104 Aug 22, 2017

@jethrogb jethrogb closed this Aug 22, 2017

@jethrogb jethrogb reopened this Aug 22, 2017

@jethrogb jethrogb force-pushed the jethrogb:filter branch from d7e6104 to 0a02e98 Aug 22, 2017

@jethrogb jethrogb force-pushed the jethrogb:filter branch 2 times, most recently from 70b8b29 to a60b5c4 Sep 11, 2017

@jethrogb

This comment has been minimized.

Copy link
Contributor Author

jethrogb commented Sep 11, 2017

Rebased, again. Can you please either accept or decline this PR so I don't have to keep rebasing it?

@KodrAus KodrAus self-requested a review Sep 12, 2017

@KodrAus

This comment has been minimized.

Copy link
Collaborator

KodrAus commented Sep 12, 2017

Thanks for this @jethrogb! Sorry about the delay, I'll give this a proper review this week. You won't need to rebase again.

@KodrAus
Copy link
Collaborator

KodrAus left a comment

Thanks again for doing this @jethrogb! I think this is a good start and will merge it in once the build is green. We might want to add some dedicated APIs over the builder before the next release so you can parse a filter for your own loggers without having to set a target explicitly.

Jethro Beekman added some commits Sep 11, 2017

@jethrogb jethrogb force-pushed the jethrogb:filter branch from a60b5c4 to dd8e1be Sep 15, 2017

@jethrogb

This comment has been minimized.

Copy link
Contributor Author

jethrogb commented Sep 15, 2017

once the build is green

Fixed. Probably should've run cargo build after the rebase.

without having to set a target explicitly

You don't have to set it. If you only call EnvLogger::matches, the target is never used.

@KodrAus

This comment has been minimized.

Copy link
Collaborator

KodrAus commented Sep 15, 2017

You don't have to set it. If you only call EnvLogger::matches, the target is never used.

Ah you're right. Having a silent target is a prerequisite to anything else we'd want to do here anyway so there's no reason to block this on thinking about other ways to expose the filtering.

Thanks again!

@KodrAus KodrAus merged commit e2b1a04 into sebasmagri:master Sep 15, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.