hashbangcode / vlad

Vlad - Vagrant LAMP Ansible Drupal
173 stars 53 forks source link

Stop nesting vlad_settings.yml in a settings directory #339

Open dixhuit opened 8 years ago

dixhuit commented 8 years ago

This behaviour is a hangover from when we used provide an additional location for drush alias files and it made sense for all this stuff to live under a single directory. It may even be a hangover from when Vlad's settings file used to be just called settings.yml.

Anyway, there's not really much need for it anymore, in fact I think it's forcing a convention in project structures that may be unwelcome. vlad_settings.yml is clearly namespaced, I don't see any issue with it sitting right beside vlad/, docroot/, composer.json or whatever else.

I could make this change myself by just amending the Vagrantfile, but it would be a breaking change. For it to be done so that the former location is still supported I'd probably need @zxaos to take a quick look as the top of the Vagrantfile has changed quite a bit since I tried editing it in earnest :)

There's gonna be a few breaking changes in the next release anyway so how do we feel about adding another?

@philipnorton42 & @zxaos I know you're busy but I'd like to hear your quick initial thoughts when you get a moment.

dixhuit commented 8 years ago

Oh and the same would go for vlad_local_settings.yml also of course.

zxaos commented 8 years ago

@danbohea I'm +1 to this.

You're right, if you don't care to support the old structure, then it's simple - you should just need to drop the /settings/prefix from VLAD_SETTINGS and VLAD_LOCAL_SETTINGS at the top of the file. I'd say we should go for this plan since any existing VM is going to need manual attention anyway.

If we do want to maintain the old structure, then we'll need to extend the paths searched in settings_files. I can take a quick stab at that if that's the way we decide to go, it shouldn't be too different from what we have there now.

One side effect that probably won't have any negative effect but might be worth mentioning is that it would place the .vagrant folder in the root rather than in the settings/ folder. This probably won't be an issue - in fact it's probably closer to what people expect given that in non-vlad situations using vagrant they'll be typing vagrant init in the project root which would result in the same thing. However this would break any existing VMs running on the dev branch (again, but same "move the folder" fix as last time).

dixhuit commented 8 years ago

One side effect that probably won't have any negative effect but might be worth mentioning is that it would place the .vagrant folder in the root rather than in the settings/ folder. This probably won't be an issue - in fact it's probably closer to what people expect given that in non-vlad situations using vagrant they'll be typing vagrant init in the project root which would result in the same thing. However this would break any existing VMs running on the dev branch (again, but same "move the folder" fix as last time).

Forgot to mention this but had considered it. I agree that this is actually an improvement. In addition to your point I'd also add that the change raises awareness about the folder's existence which is useful in understanding how this all fits together. I'd also add that the folder is a hidden directory so to some users this is going to be of no noticeable consequence.

dixhuit commented 8 years ago

@philipnorton42 I know we're all busy but I'm edging closer & closer to just implementing this. Do you have any concerns here? A simple :+1: or :-1: will do for now! :D