search

woohoolabs / harmony

A simple and flexible PHP middleware dispatcher based on PSR-7, PSR-11, and PSR-15
MIT License
164 stars 16 forks source link

disable Xdebug when not needed #10

Closed xabbuh closed 8 years ago

kocsismate commented 8 years ago

Hey!

Thanks for the patch! Maybe you noticed that I've just removed this line a couple of days ago after a discussion in issue #9 (specifically here: https://github.com/woohoolabs/harmony/commit/6994508ad8f8d9ddcb46631c129edd32e0f129a8).

When looking at the PHP 5.6 build for a recent commit of mine (https://travis-ci.org/woohoolabs/harmony/jobs/129048700) and yours (https://travis-ci.org/woohoolabs/harmony/jobs/129045688) I can't really see a big difference in performance.

Without XDebug:

However I know these numbers aren't really representative because Travis isn't reliable.

Meanwhile I noticed that lately, Travis runs compose self-update automatically (I believe they didn't do it in the past!), so I removed it from the before_install section, creating a conflict with your PR. :S

Although it seems that there is not a big need for disabling XDebug, if you resolve the conflict, I'll merge your PR. :) Sorry for the inconvenience.

kocsismate commented 8 years ago

@sagikazarmark I gained some insights about Xdebug and realized that composer self-install is not needed anymore in the Travis config!

sagikazarmark commented 8 years ago

The reason why I usually have it is that composer gets new features from time to time, however it takes a considerable amount of time for Travis to update composer. This way you always have a fresh one. Is there anything new on this front I don't know about?

kocsismate commented 8 years ago

Yes, I also thought that Travis doesn't update Composer often, but check my first comment where you can see on the linked build status report (https://travis-ci.org/woohoolabs/harmony/jobs/129048700#L135) that composer self-update runs twice: on line 135 and line 162.

The first one is when Travis provisions the container, and the second one is when I run it in the before_install section. After I removed this section, Composer doesn't get updated twice: https://travis-ci.org/woohoolabs/harmony/jobs/129120810#L135

sagikazarmark commented 8 years ago

Ah, cool, good to know. Thanks for the heads up.

xabbuh commented 8 years ago

@kocsismate Yes, it doesn't save you much time with the current test suite (the differences between the several builds are mainly related to the load of the underlying machines that Travis uses I guess). Though it doesn't hurt and has positive effects in case the test suite is expanded in the future. :)

kocsismate commented 8 years ago

And at least I could discover that composer self-update can also be omitted! So thank you for your PR!