humanmade / Salty-WordPress

A flavorful way to manage your entire WordPress stack.
126 stars 22 forks source link

Install XDebug for vagrant machine #128

Closed roborourke closed 8 years ago

roborourke commented 8 years ago

Makes sure xdebug is installed on a vagrant machine

Optimises the xdebug config - profiler settings caused hdd to fill up with unnecessary amount of cachegrind logs

roborourke commented 8 years ago

@joehoyle didn't realise xdebug wasn't installed on local salty vagrant boxes. This addition means I can finish off the post & screencast for getting it set up for those who are interested

joehoyle commented 8 years ago

@sanchothefat I think this has come up before, and we basically decided we didn't want it in by default, but could be added via the local.sls file, see https://github.com/humanmade/Salty-WordPress/issues/87 and https://github.com/humanmade/Salty-WordPress/pull/24

roborourke commented 8 years ago

@joehoyle Okey dokey. Out of interest do you know why was that decision made? Seems weird for a dev machine. I'll just disagree quietly in a corner at any rate.

dan-westall commented 8 years ago

@joehoyle I'd argue that xdebug should be considered for inclusion in satly, i agree with the general gist of #24 local.sls file so everyone can have their own local mods in a sustainable way. and for some of the more exotic combinations people might need that makes sense, but xdebug isn't exotic, its a stable, even an essential part of a developer toolbox. So everyone who develops on salty will at some point will have to waste time getting xdebug running .

pdewouters commented 8 years ago

I agree, local.sls is good for really special cases that nobody else would benefit from, but adding xdebug has lots of advantages for most devs. What is the argument against including by default? I think it's worth having another discussion

joehoyle commented 8 years ago

Ok, concerns voiced :) So my initial concern around having xdebug on by default is the function overloading it does with things like var_dump. I'm pretty ignorant to configuring that stuff, but if someone can educate me such that we don't have to have the "default" xdebug visual stuff on, I'm happy to go with this.

My initial rejection of this idea was focused on not wanting to sway from the "default" setup of PHP and how errors are handled etc, to be less opinionated about it.

joehoyle commented 8 years ago

I'll just disagree quietly in a corner at any rate.

Also, please don't do this :)

roborourke commented 8 years ago

@joehoyle ah cool - hopefully p easy to do, I always use log files rather the visual stuff at any rate as the stack traces are useful. Aces, will update the PR shortly

roborourke commented 8 years ago

Few options:

xdebug.overload_var_dump = 0

or we change the html_errors setting to

html_errors = 0

in the php.ini as it uses that value.

danielbachhuber commented 8 years ago

Could we introduce a feature flag system, so users could optionally provision with features like xdebug?

roborourke commented 8 years ago

Judging by the amount it's come up previously not having it on vagrant is the less popular requirement so it makes sense to me to have it by default.

If we do a feature flag system then more advanced devs like yourself who don't want/need xdebug could use that to tweak how salty is provisioned to suit. We should cater the environment towards the majority of users and most new hires at HM use an IDE with remote debugging features and want to use it.

I don't really understand the reasons against having it available on a dev environment still compared to the benefits. What's the actual impact on your process / workflow beyond not liking the var_dump() overloading?

joehoyle commented 8 years ago

I think for Xdebug, we should just add it with the above PHP directives @sanchothefat mentioned. I don't think this needs to be featured flagged, as mentioned above several times, there's little downside and a lot of upside for many people.

Feature flags could be interesting for other things, though I'm not totally sure how this could work / if there is prior art for passing flags to vagrant provision for example.

roborourke commented 8 years ago

@joehoyle last tweak done! Good to go?

pdewouters commented 8 years ago

giphy