-
Notifications
You must be signed in to change notification settings - Fork 54
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
Better profiler (based on PR #29) #57
base: master
Are you sure you want to change the base?
Conversation
Notice some tests are failing... Could use some help reviewing/fixing those, its a bit unfamiliar territory for me :) |
I just saw the failed log. I'm not sure I will able to help you. But thanks a lot for your effort @rvgate ! |
Could you rebase? I've fixed the tests in 5f516c7. (There were 2 mock plugins accidentally added to the clients, and guzzle/guzzle#576 changed the behaviour from doing nothing when the queue is empty to throwing an exception (hence the tests failing here as one of the plugins never has responses added).) |
@thewilkybarkid Rebased from 5f516c7 but still failing... what am I missing here?? |
@ludofleury Thanks for the fix! @thewilkybarkid Everything seems to be alright now, anything you want me to do before merging? |
👏 |
@thewilkybarkid ping |
@thewilkybarkid ping2 |
Any updates on a possible merge? It has been so long... is this project still being continued? ping @thewilkybarkid |
…ofiler features from @ludofleur bundle and @leaseweb apicallerbundle
@thewilkybarkid Did another rebase based off the changes you did a few weeks ago, since it has been a while since the pull request was opened... any plans of merging this? As an addition, it now also collects the postFields. |
@ddeboer @zerrvox @mattvick @alex-pex @tristanbes @adrian-ludwig @bendavies @adrienbrault @ludofleury @thewilkybarkid @mevdschee Sorry for pinging all of you, you were mentioned in PR #29 willing to have this pull request merged... Im not sure what to do here... it has been a few months now and no activity or any expectations on a possible merge... What happened to the great plans of collaborating with other guzzle bundles (Issue #28)? I could release this pull request as a new bundle at @leaseweb but that defeats the purpose of this whole PR. Lets collaborate! |
Agree. |
Hey @thewilkybarkid, @rvgate did some quite of work here and it seems that this PR could be merged as is. He fixed everything you requested & added some nice additions. Could you provide an answer about your position by any chance ? |
+1 |
2 similar comments
+1 |
+1 |
It seems this project is abandoned. 😢 |
@meeprophone ping Anyone still maintaining this project???? |
I really appreciate this initiative to bring the bundles together, it is a very good idea. Unfortunately none of the maintainers (including @thewilkybarkid, Chris Wilkinson) is responding in the past 9 months, which is sad. I suggest that somebody forks this project to keep it alive. |
I agree with @mevdschee, @mtdowling seems to go forward with Guzzle (already version 5) so we can't really push this fork behind the official namespace/organisation. I think @rvgate is pretty active on this project with his team, so maybe he should fork this one and take the lead on the repo. If ever someone plan to upgrade to Guzzle 5, this could be the time where we should consider again guzzle organisation. |
@mevdschee @ludofleury @rvgate If any of you wish to contribute, there's always CsaGuzzleBundle, which supports Guzzle 4, 5 (on the 1.3 branch) and 6 (on the master branch). Btw, a few features won't ever get ported from this bundle, as Guzzle services and guzzle command have not been upgraded to support Guzzle 6. /cc @mtdowling |
Not sure if I had to create a new pull request here or at ludofleury:ludofleury.guzzle-profiler, related to pull request #29.
Basically what I did was use his pull request and changed the profiler to confirm with the comments in pull request #29. Here's a quick summary;
That's about it... All credits should go to @ludofleury for providing us the base that allowed me to do these changes.
Hope you like it!