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

Better profiler (based on PR #29) #57

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

rvgate
Copy link

@rvgate rvgate commented May 19, 2014

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;

  • Changed logo back to the original misd-service-development-guzzle-bundle
  • Total times directly visible in the debugger toolbar and debugger menu (To make sure its consistent with other bundles that do this)
  • Requests are listed in an UL, like some other debuggers (Doctrine, Logger)
  • Changed the content to be somewhat the same like the LeaseWeb ApiCallerBundle, splitting information into timing, request and response per guzzle call.
  • Vanilla JS for toggling div display
  • Removed all other dependencies (jquery, fontawesome, syntax highlighting)
  • For viewing body content, you can chose raw, html or parsed (json), this idea came from ApiCallerBundle aswell

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!

@rvgate
Copy link
Author

rvgate commented May 19, 2014

Notice some tests are failing... Could use some help reviewing/fixing those, its a bit unfamiliar territory for me :)

@ludofleury
Copy link

I just saw the failed log. I'm not sure I will able to help you. But thanks a lot for your effort @rvgate !
It's awesome.

@thewilkybarkid
Copy link
Contributor

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).)

@rvgate
Copy link
Author

rvgate commented May 23, 2014

@thewilkybarkid Rebased from 5f516c7 but still failing... what am I missing here??

@rvgate
Copy link
Author

rvgate commented Jun 5, 2014

ping @thewilkybarkid @ludofleury

@rvgate
Copy link
Author

rvgate commented Jun 5, 2014

@ludofleury Thanks for the fix!

@thewilkybarkid Everything seems to be alright now, anything you want me to do before merging?

@ludofleury
Copy link

👏

@rvgate
Copy link
Author

rvgate commented Jun 10, 2014

@thewilkybarkid ping

@rvgate
Copy link
Author

rvgate commented Jun 17, 2014

@thewilkybarkid ping2

@rvgate
Copy link
Author

rvgate commented Jul 27, 2014

Any updates on a possible merge? It has been so long... is this project still being continued?

ping @thewilkybarkid

@rvgate
Copy link
Author

rvgate commented Jan 3, 2015

@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.

@rvgate
Copy link
Author

rvgate commented Jan 5, 2015

@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!

@ludofleury
Copy link

Agree.

@ludofleury
Copy link

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 ?

@mevdschee
Copy link

+1

2 similar comments
@thecodeassassin
Copy link

+1

@zerrvox
Copy link

zerrvox commented Jan 6, 2015

+1

@mevdschee
Copy link

It seems this project is abandoned. 😢

@rvgate
Copy link
Author

rvgate commented Feb 8, 2015

@meeprophone ping
@NMattin ping
@rh389 ping
@StuBez ping
@thewilkybarkid ping

Anyone still maintaining this project????

@mevdschee
Copy link

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.

@ludofleury
Copy link

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.

@csarrazi
Copy link

@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

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.

7 participants