Certain cruical variables are still used when config comments suggest they won't be

[dixhuit]

Examples:

# Set 'build_makefile' to 'false' and this to 'true' if you are using a
# composer based site deployment strategy.
build_composer: false
drupal_composer_path: "{{ config_dir }}/drupal.composer.json"
drupal_composer_install_dir: "/var/www/drupalvm/drupal"
drupal_composer_dependencies:
  - "drupal/devel:8.*"

drupal_composer_install_dir is still used (see below) even if I'm not using composer.

# Settings for installing a Drupal site (if 'install_site:' is 'true').
drupal_major_version: 8
drupal_core_path: "{{ drupal_composer_install_dir }}/web"
drupal_domain: "{{ vagrant_hostname }}"
drupal_site_name: "Drupal"
drupal_install_profile: standard
drupal_enable_modules: [ 'devel' ]
drupal_account_name: admin
drupal_account_pass: admin
drupal_mysql_user: drupal
drupal_mysql_password: drupal
drupal_mysql_database: drupal

drupal_core_path is still used elsewhere even if I set install_site to false. The mysql settings are still used too.

These variables are crucial to a setup but it's made out that they are entirely optional depending on how you choose to have DVM build your site if at all.

[oxyc]

As of 3.1.0 all variabels are included by default even if you don't have them set in your config.yml. This means that as long as you don't change the values to something invalid, it works out of the box. It's more about overriding now.

In the docs we still suggest that people copy the default.config.yml to config.yml and make changes, but as of 3.1.0, you can just remove or comment out the values that you don't want to override, hence they are optional.

Ideally some of these would be moved into geerlingguy.drupal at some point.

Before 3.1.0 I would definitely have agreed with you. It took some time to figure out which variables were still required when I used this VM for Wordpress sites. But now with default.config.yml always being loaded I don't feel it's an issue anymore. How do you think it should be clarified further?

[dixhuit]

I'm aware of the change in 3.1.0 and fully support it - it's more like Vlad which I'm more familiar with and individual project config is much clearer to read due to there not being a sea of variables to scan.

But default.config.yml acts as a form of documentation: users will still look to it as reference or an example at least, developers looking to contribute will also use the comments therein as initial guidance. In its present state its misleading considering the examples I've flagged above.

IMO it would be clearer if the crucial variables were simply separated out from the comments that suggest that they're optional.

Adjusted example:

# Required Drupal settings
drupal_core_path: "{{ drupal_composer_install_dir }}/web"
drupal_mysql_user: drupal
drupal_mysql_password: drupal
drupal_mysql_database: drupal

# Settings for installing a Drupal site if 'install_site:' is 'true'
drupal_major_version: 8
drupal_domain: "{{ vagrant_hostname }}"
drupal_site_name: "Drupal"
drupal_install_profile: standard
drupal_enable_modules: [ 'devel' ]
drupal_account_name: admin
drupal_account_pass: admin

Happy to provide a PR.

[geerlingguy]

That would be acceptable—reordering so it's more clear.

[geerlingguy]

Fixed via merged PR.