{"action":"created","issue":{"url":"https://api.github.com/repos/roots/trellis/issues/765","repository_url":"https://api.github.com/repos/roots/trellis","labels_url":"https://api.github.com/repos/roots/trellis/issues/765/labels{/name}","comments_url":"https://api.github.com/repos/roots/trellis/issues/765/comments","events_url":"https://api.github.com/repos/roots/trellis/issues/765/events","html_url":"https://github.com/roots/trellis/pull/765","id":208702848,"number":765,"title":"Consolidate multisite config into env vars only","user":{"login":"fullyint","id":7338513,"avatar_url":"https://avatars.githubusercontent.com/u/7338513?v=3","gravatar_id":"","url":"https://api.github.com/users/fullyint","html_url":"https://github.com/fullyint","followers_url":"https://api.github.com/users/fullyint/followers","following_url":"https://api.github.com/users/fullyint/following{/other_user}","gists_url":"https://api.github.com/users/fullyint/gists{/gist_id}","starred_url":"https://api.github.com/users/fullyint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fullyint/subscriptions","organizations_url":"https://api.github.com/users/fullyint/orgs","repos_url":"https://api.github.com/users/fullyint/repos","events_url":"https://api.github.com/users/fullyint/events{/privacy}","received_events_url":"https://api.github.com/users/fullyint/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-02-19T08:20:00Z","updated_at":"2017-02-19T22:18:31Z","closed_at":"2017-02-19T22:18:31Z","pull_request":{"url":"https://api.github.com/repos/roots/trellis/pulls/765","html_url":"https://github.com/roots/trellis/pull/765","diff_url":"https://github.com/roots/trellis/pull/765.diff","patch_url":"https://github.com/roots/trellis/pull/765.patch"},"body":"This PR is an alternative to #754. Fixes #554.\r\n\r\nAs in #754, this PR proposes the usage of env vars for `MULTISITE` and `SUBDOMAIN_INSTALL` as part of changes that prevent the `wp core is-installed` check from failing ([solution from](https://github.com/roots/trellis/pull/754#issuecomment-280131261) @QWp6t).\r\n\r\n### One multisite config only\r\n\r\nThe difference from #754 is that this PR proposes to use the new env vars to control all of the Trellis multisite setup. This feels conceptually cleaner than how #754 uses the old `multisite.enabled` and `multisite.subdomains` configs as indirect controls of the configs WP uses directly (the env vars).\r\n\r\nThe changes from current master are not drastic. Changes are quite simple, like this example:\r\n```diff\r\n-    when: project.multisite.enabled | default(false)\r\n+    when: site_env.multisite | default(false)\r\n```\r\n\r\n### Breaking change to `wordpress_sites`\r\n\r\nThis would be a breaking change to the structure/expectations of `wordpress_sites`. The majority of the edits in this PR are validations of the multisite configs, with messages to help users make the transition. Aside from these validations and messages, there are no new lines of code, only simple changes to existing lines, as in the diff above.\r\n\r\nHere is the message users would see if the old `multisite.enabled` setting is present but `false`:\r\n```\r\nWarning! The old `multisite.enabled` and `multisite.subdomains`\r\nsettings are now ignored. Multisite must be configured ONLY in\r\nthe `env` vars of `group_vars/production/wordpress_sites.yml`.\r\n\r\nYou don't have multisite enabled for domain.com so you may completely omit\r\nany multisite config for domain.com in\r\n`group_vars/production/wordpress_sites`\r\n\r\nSee https://roots.io/trellis/docs/multisite/\r\n```\r\n\r\nOr if `multisite.enabled` setting is present and `true`:\r\n```\r\nWarning! The old `multisite.enabled` and `multisite.subdomains`\r\nsettings are now ignored. Multisite must be configured ONLY in\r\nthe `env` vars of `group_vars/production/wordpress_sites.yml`.\r\n\r\nwordpress_sites:\r\n  domain.com:\r\n    ...\r\n    env:\r\n      multisite: true                      # default: false\r\n      subdomain_install: true              # default: false\r\n\r\nSee https://roots.io/trellis/docs/multisite/\r\n```\r\n\r\nThe playbook will warn and continue if the new `env.multisite` is present; Presumably the user has consciously set an appropriate value and provisioning/deployment will be appropriate. However, if the old `multisite.enabled` is present as `true` and `env.multisite` is not defined, the playbook will fail because it will not provision/deploy a multisite setup, as was apparently intended.\r\n\r\n### Simpler than #754\r\n\r\nCompared to #754, this PR allows users who aren't using multisite to simplify their `wordpress_sites` to omit any `multisite` config without getting an `undefined variable` warning, as in current master. In addition, this PR avoids having to set several new default env vars (as were required in #754).\r\n\r\n### Less chance for error than #754 \r\n\r\n#754 leaves it possible for users to set `multisite.enabled: true` and `env.multisite: false`, which entanglement might happen after a new user's iterations of trying out trellis and multisite. This would yield nginx confs set up for multisite, but WP constants _not_ set for multisite. The current PR avoids the problem by having only one possible place for the config.\r\n\r\n### Required changes to docs\r\n\r\nTo illustrate what would change for users, here are the primary changes to the [multisite docs](https://roots.io/trellis/docs/multisite/):\r\n\r\n1. Suggest defining the constants in a way that loads the env vars:\r\n\r\n```\r\ndefine('MULTISITE', env('MULTISITE') ?? true);\r\ndefine('SUBDOMAIN_INSTALL', env('SUBDOMAIN_INSTALL') ?? true); // Set to false if using subdirectories\r\n```\r\n\r\n2. Switch to illustrating the new env vars instead of `multisite.enabled`.\r\n\r\n```\r\nwordpress_sites:\r\n  example.com:\r\n    ...\r\n    env:\r\n      multisite: true\r\n      subdomain_install: true  # omit or set to false if using subdirectories\r\n```"},"comment":{"url":"https://api.github.com/repos/roots/trellis/issues/comments/280954270","html_url":"https://github.com/roots/trellis/pull/765#issuecomment-280954270","issue_url":"https://api.github.com/repos/roots/trellis/issues/765","id":280954270,"user":{"login":"fullyint","id":7338513,"avatar_url":"https://avatars.githubusercontent.com/u/7338513?v=3","gravatar_id":"","url":"https://api.github.com/users/fullyint","html_url":"https://github.com/fullyint","followers_url":"https://api.github.com/users/fullyint/followers","following_url":"https://api.github.com/users/fullyint/following{/other_user}","gists_url":"https://api.github.com/users/fullyint/gists{/gist_id}","starred_url":"https://api.github.com/users/fullyint/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fullyint/subscriptions","organizations_url":"https://api.github.com/users/fullyint/orgs","repos_url":"https://api.github.com/users/fullyint/repos","events_url":"https://api.github.com/users/fullyint/events{/privacy}","received_events_url":"https://api.github.com/users/fullyint/received_events","type":"User","site_admin":false},"created_at":"2017-02-19T22:18:31Z","updated_at":"2017-02-19T22:18:31Z","body":"Closed by better solution in #766.\r\n\r\nIn my subconscious, I had this misconception:\r\n>Trellis now controls WP's configs/constants. And if env vars are the means of controlling WP configs, why not avoid the indirection of `multisite.enabled` and just use env vars directly?\r\n\r\nAfter internal discussion, I realize that Trellis is _not_ in the business of controlling WP's constants, of course. Rather, the constants and env vars came up just as part of an attempted workaround, a workaround that #766 renders unnecessary."}}
{"actor":{"display_login":"fullyint"}}