Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CRM-18231 - Cleanup environment example in civicrm.settings.php.template #10972

Merged
merged 3 commits into from
Sep 11, 2017

Conversation

totten
Copy link
Member

@totten totten commented Sep 11, 2017

Overview

Update the environment example in civicrm.settings.php.template.

Before

  • If you uncomment and tweak the example environment, it crashes -- because class-constants don't make sense in this context.
  • If you fix that, then the setting still doesn't take effect -- because you need to uncomment the global $civicrm_setting declaration.

After

  • If you uncomment and tweak the example environment, it works.

Technical Details

To test this, I would iteratively edit the config file and then run

cv ev 'return [Civi::settings()->get("environment"), CRM_Core_Config::environment()];'

Ping @seamuslee001 @monishdeb @eileenmcnaughton .


…example

The expression `CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME` does not
work within `civicrm.settings.php` because the classloader has not been
initialized yet.  This file is just supposed to store the data needed for
bootstrap.
When using the example snippets in the settings file, it's easy to
accidentally leave `global $civicrm_setting` commented out -- which is
fairly confusing/mysterious way to have your settings ignored.

There's no cost to declaring `global $civicrm_setting` by default in new
config files.
@totten totten changed the base branch from master to 4.7.25-rc September 11, 2017 18:52
@totten totten removed the master label Sep 11, 2017
Copy link
Contributor

@seamuslee001 seamuslee001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me. Tim can you just confirm the group of the setting?

// $civicrm_setting[CRM_Core_BAO_Setting::DEVELOPER_PREFERENCES_NAME]['environment'] = 'Production';
* via the browser.
*/
// $civicrm_setting['domain']['environment'] = 'Production';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@totten isn't it 'Developer'? Not domain?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want the verbose/legacy style, then you'd need to lookup the exact group name. In this case, it would be Developer Preferences.

In 4.7.0+, all those things are just aliases for domain: https://docs.civicrm.org/dev/en/master/api/changes/#470-global-civicrm_setting-multiple-changes

@eileenmcnaughton eileenmcnaughton merged commit 76f6dea into civicrm:4.7.25-rc Sep 11, 2017
@totten totten deleted the 4.7.25-rc-tpl branch September 11, 2017 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants