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

Feat/noterm #91

Merged
merged 6 commits into from
Oct 18, 2016
Merged

Feat/noterm #91

merged 6 commits into from
Oct 18, 2016

Conversation

Fiedzia
Copy link
Contributor

@Fiedzia Fiedzia commented Oct 17, 2016

Remove support for logging to terminal, as well as using color and text attributes,
relying simply on log module to deliver the message. I've tested it with stdio_logger create.
This is based on Alexander Irbis fork.

@Fiedzia Fiedzia mentioned this pull request Oct 17, 2016
@Hoverbear
Copy link

Hoverbear commented Oct 17, 2016

Hi! Thanks for this! Would you mind fixing up the examples as well? (This is why the tests are failing)

Copy link

@Hoverbear Hoverbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a couple of tiny changes in Cargo.toml and the README.md if you don't mind! :)

Then we can publish a new (breaking) version.

description = "Logging middleware for the Iron framework."
repository = "https://github.com/iron/logger"
keywords = ["iron", "web", "logger", "log", "timer"]
license = "MIT"

[dependencies]
iron = { version = "0.4", default-features = false }
term = "0.4"
log = "*"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current version of log is 0.3.6. We can't use * specifiers in published crates so this needs to be changed.

- Format strings can specify fields to be logged as well as ANSI terminal colors and attributes.
Logger prints request and response information to the configured log, using either a default format or a custom format string.

Format strings can specify fields to be logged (ANSI terminal colors and attributes is no longer supported).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be great if we could attach a reason link if we're actually going to say that. Otherwise I'd just leave it out.

Eg.

Format strings can specify fields to be logged (ANSI terminal colors and attributes is no longer supported since #82.)

Some(c) => { try!(self.term.fg(c)); }
None => {},
let lg = format.iter().map(|unit| render(&unit.text)).collect::<Vec<String>>().join("");
info!("{}", lg);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, glad to see the use of log. :)

@Hoverbear
Copy link

Hoverbear commented Oct 18, 2016

This PR relates to, and closes, #82.

@Hoverbear
Copy link

Thanks! Looks great! =D

@Hoverbear Hoverbear merged commit 633c5ee into iron:master Oct 18, 2016
@Fiedzia Fiedzia deleted the feat/noterm branch October 18, 2016 07:21
@Fiedzia
Copy link
Contributor Author

Fiedzia commented Oct 18, 2016

Thanks for reviewing and merging, we should update documentation to indicate that some log implementation is required (and point to some).

@Hoverbear
Copy link

Can you open a new issue for that? :)

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