search

expressjs / response-time

Response time header for node.js
MIT License
477 stars 73 forks source link

Add option to disable 'ms' appendix #5

Closed analytically closed 10 years ago

dougwilson commented 10 years ago

Yea, I can add that. Any particular reason you need it (just trying to justify feature bloat is all)?

analytically commented 10 years ago

Actually, nevermind. By default http response timings are in seconds. So the practice of adding 'ms' is actually better when it's not seconds.

analytically commented 10 years ago

OTOH dropbox doesn't include ms: https://www.dropbox.com/ x-server-response-time:141

dougwilson commented 10 years ago

Well, I could potentially allow configuration for the unit you want it in + if you want the units appended. Would that be something you want?

dougwilson commented 10 years ago

OTOH dropbox doesn't include ms: https://www.dropbox.com/ x-server-response-time:141

That sounds like a good enough case to me. Any thoughts on my latest comment as well?

analytically commented 10 years ago

Changing the unit isn't an issue for us. When response times are in the seconds, that's a problem! :-)

dougwilson commented 10 years ago

haha, sounds good :) I should have a new release for you tonight (US time).

analytically commented 10 years ago

Cool, thanks!

dougwilson commented 10 years ago

Let me know what you think of the implementation on master.

analytically commented 10 years ago

I would maybe allow suffix to be a boolean or string? If string, use that as suffix (eg. millis instead of ms)?

dougwilson commented 10 years ago

Yea, I thought about that, but didn't want to preclude the future possibility to adjust the SI unit of the output or something. Basically I was trying to be conservative on that front, IDK :)

analytically commented 10 years ago

Haha ok cool. In that case, looks great.

dougwilson commented 10 years ago

Alrighty, published as 2.2.0