search

Varying-Vagrant-Vagrants / VVV

An open source Vagrant configuration for developing with WordPress
https://varyingvagrantvagrants.org
MIT License
4.54k stars 847 forks source link

Add backward compatible for db_backup option #2502

Closed emgk closed 2 years ago

emgk commented 3 years ago

Did we intentionally extended db_backup option here https://github.com/Varying-Vagrant-Vagrants/VVV/pull/2478 (couldn't find useful doc related to this change), pulling these changes to the local setup can break the purpose of db_backup, because its configured in an old way db_backup: false in vvv/config.yml file, can we add backward compatibility in following files?

https://github.com/Varying-Vagrant-Vagrants/VVV/blob/b4d2655bf01120e32cdfc6251af26a2c009eeccc/config/homebin/vagrant_destroy#L21 https://github.com/Varying-Vagrant-Vagrants/VVV/blob/b4d2655bf01120e32cdfc6251af26a2c009eeccc/config/homebin/vagrant_halt#L21 https://github.com/Varying-Vagrant-Vagrants/VVV/blob/b4d2655bf01120e32cdfc6251af26a2c009eeccc/config/homebin/vagrant_suspend#L21

Thanks :pray:

Mte90 commented 3 years ago

So you mean add a code that check the previous parameter for old VVV instance that didn't updated the settings? If yes , it is something that we need to do.

tomjn commented 3 years ago

@Mte90 this was a part of the original spec, we can't break things for existing users, it should never have been merged if it that wasn't the case

tomjn commented 3 years ago

@Mte90 I'm labelling this OMGWTFBBQ level high priority

tomjn commented 3 years ago

@emgk that PR isn't related to this, it fixes a YAML syntax error

tomjn commented 3 years ago

https://github.com/Varying-Vagrant-Vagrants/VVV/pull/2346 is the PR that implemented this

Based on the what the OP says It looks like nobody did the testing I asked for:

https://github.com/Varying-Vagrant-Vagrants/VVV/pull/2346#issuecomment-821257734

Mte90 commented 3 years ago

I did a pr but require some testings.