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

Added field to print the request time. #89

Merged
merged 1 commit into from
Oct 14, 2016
Merged

Added field to print the request time. #89

merged 1 commit into from
Oct 14, 2016

Conversation

txgruppi
Copy link
Contributor

@txgruppi txgruppi commented Sep 4, 2016

Added the {request-time} field which prints the time when the
request ocurred using the format "%Y-%m-%dT%H:%M:%S.%fZ%z".

Fix #69

Added the {request-time} field which prints the time when the
request ocurred using the format "%Y-%m-%dT%H:%M:%S.%fZ%z".

Fix #69
@@ -29,7 +29,7 @@ impl Logger {
/// Create a pair of `Logger` middlewares with the specified `format`. If a `None` is passed in, uses the default format:
///
/// ```ignore
/// {method} {uri} -> {status} ({response_time} ms)
/// {method} {uri} -> {status} ({response-time} ms)

Choose a reason for hiding this comment

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

Thanks for fixing this!

@@ -48,18 +48,18 @@ impl Logger {
}

struct StartTime;
impl Key for StartTime { type Value = time::Instant; }
impl Key for StartTime { type Value = time::Tm; }
Copy link

@Hoverbear Hoverbear Oct 14, 2016

Choose a reason for hiding this comment

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

Are these changes to StartTime (and the ones below) required for this PR? Otherwise I'd prefer to see them in another PR.

Copy link
Contributor Author

@txgruppi txgruppi Oct 14, 2016

Choose a reason for hiding this comment

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

The problem with time::Instant is that it doesn't have a format function.

It may be because I don't understand Rust 100% yet but I can't find a way to do it without time::Tm.

Choose a reason for hiding this comment

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

Ahhhh okay this makes sense.

@@ -11,3 +11,4 @@ license = "MIT"
[dependencies]
iron = { version = "0.4", default-features = false }
term = "0.4"
time = "0.1.35"

Choose a reason for hiding this comment

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

Looks like std::time exists now. https://doc.rust-lang.org/std/time. However I'm not sure it did at the time of this PR. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Hoverbear
Copy link

Hey! Sorry it took so long to get around to this! Let's get this merged, it's on my radar now!

@Hoverbear
Copy link

Thanks so much for your contribution! :)

@Hoverbear Hoverbear merged commit 5d34d84 into iron:master Oct 14, 2016
@reem reem removed the in progress label Oct 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants