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

civicrm.settings.php.template - Simplify examples of $civicrm_setting #16636

Merged
merged 1 commit into from
Feb 27, 2020

Conversation

totten
Copy link
Member

@totten totten commented Feb 27, 2020

Overview

This simplifies the examples in civicrm.settings.php.template.

Before

Every reference to $civicrm_setting uses a different, English-looking group-name.

$civicrm_setting['URL Preferences']['imageUploadURL']

After

All examples use domain.

$civicrm_setting['domain']['imageUploadURL']

Comments

Since CiviCRM v4.7, expessions like $civicrm_setting['URL Preferences'] have been aliases for $civicrm_setting['domain']. (See SettingsManager::parseMandatorySettings().)

These examples were initially kept in the verbose 4.6 format so that users of 4.6 and 4.7 could continue to exchange examples with each other. But 4.6 and 4.7 are pretty old, so I don't think that's an issue anymore. We're now firmly in the 5.x world.

What does still matter is intuition - if the examples set an expectation that you should put things under buckets like URL Preferences, then it implies that you should be thinking about those buckets. (Does the ext_repo_url belong under Extension Preferences or URL Preferences? Can I make up new groups? If I know the name used by the Setting API, then how do I figure out the group-name used for the $civicrm_setting override? ... The answer to all of these questions is that it doesn't matter because they're really the same group.)

(Aside: For the curious... there are two namespaces, domain and contact. But we rarely use contact. It would be possible but exceedingly unusual to override a contact preference via civicrm.settings.php. So in practice everything is domain.)

Overview
--------

This simplifies the examples in `civicrm.settings.php.template`.

Before
------

Every reference to `$civicrm_setting` uses an English-looking group-name like

``php
$civicrm_setting['URL Preferences']['imageUploadURL'] = 'http://example.com/example-image-upload-url';
```

After
-----

The varying group names are just `domain`.

```php
$civicrm_setting['domain']['imageUploadURL'] = 'http://example.com/example-image-upload-url';.
```

Comments
--------

Since CiviCRM v4.7, expessions like `$civicrm_setting['URL Preferences']` have been aliases
for `$civicrm_setting['domain']`.  (See `SettingsManager::parseMandatorySettings()`.)

These examples were initially kept in the verbose 4.6 format so that users of 4.6 and 4.7
could continue to exchange examples with each other.  But 4.6 and 4.7 are pretty old, so I
don't think that's an issue anymore. We're now firmly in the 5.x world.

What does still matter is intuition - if the examples set an expectation that you should put
things under buckets like `URL Preferences`, then it implies that you should be thinking about
those buckets.  (Does the `extensionsURL` belong under `URL Preferences` or `Extension
Preferences`?  Should a search-related setting defined by an extension go under `Search
Preferences` or `Extension Preferences`?  Can I make up new groups?  If I know the name used
by the Setting API, then how do I figure out the group-name used for the `$civicrm_setting`
override?  ...  The answer to all of these questions is that it doesn't matter because they're
really the same group.)
@civibot
Copy link

civibot bot commented Feb 27, 2020

(Standard links)

@civibot civibot bot added the master label Feb 27, 2020
@seamuslee001
Copy link
Contributor

Change makes sense test fail is unrelated merging

@seamuslee001 seamuslee001 merged commit a28ff21 into civicrm:master Feb 27, 2020
@totten totten deleted the master-settings-tpl branch February 28, 2020 22:21
@eileenmcnaughton
Copy link
Contributor

@ejegg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants