Skip to content

Adding time for the request #5

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

Merged
merged 5 commits into from
Apr 24, 2018
Merged

Adding time for the request #5

merged 5 commits into from
Apr 24, 2018

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Jan 6, 2018

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Related tickets
Documentation if this is a new feature, link to pull request in https://github.com/php-http/documentation that adds relevant documentation
License MIT

What's in this PR?

We should log the time it takes to make a request

If we think this is something we want Im happy to complete this PR.

To Do

  • Changelog
  • Tests

@dbu
Copy link
Contributor

dbu commented Jan 7, 2018

sounds valuable to me, lets do it!

@Nyholm
Copy link
Member Author

Nyholm commented Mar 28, 2018

I found this PR which I did not complete. Im not sure how I could make PHPSpec allow me to add a random integer like I did. I've searched the manual extensively but found nothing.

@nicholasruunu
Copy link
Contributor

@Nyholm This is how you would handle it: nicholasruunu@b708f38

But the "Received response" info calls are actually not being made when you call handleRequest().

@Nyholm
Copy link
Member Author

Nyholm commented Mar 28, 2018

Awesome Nicholas. Thank you. Will finish this PR tonight.

@Nyholm
Copy link
Member Author

Nyholm commented Mar 28, 2018

The test showed that my implementation was flawed. Thanks again for helping me.

I've used your commit and I've updated the PR. Im now ready for a review

@joelwurtz
Copy link
Member

Nice one. i'm also wondering if we should use Stopwatch in the future if present, and puts this informations in the context ?

@Nyholm
Copy link
Member Author

Nyholm commented Apr 24, 2018

Thank you for the review. Hm.. Isn't that done already if you use the Symfony Bundle?

@Nyholm Nyholm merged commit 262954e into master Apr 24, 2018
@Nyholm Nyholm deleted the time branch April 24, 2018 11:26
@joelwurtz
Copy link
Member

It was more using Stopwatch implementation instead of microtime if available, but not really necessary

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