Move log and compilation dirs from "Runtime" to "Paths" #14718

totten · 2019-07-03T05:41:40Z

Overview

This is an update to the path-handling convention for data folders that allows more intuitive management of the file-paths. It aims to:

Provide clearer separation between public and private data folders.
Provide backward-compatiblity with the vast majority of installations.
Provide better tunability for data folders -- so that sysadmins and installers can manage paths precisely in civicrm.settings.php (without as much concern for the wonky/legacy/default computations).

This PR continues previous work (circa 4.7.x) which added $civicrm_paths to civicrm.settings.php. It extends the work to address the "private" folders, creating a hierarchy of configurable paths.

Variable	Example Value	Default Value
`civicrm.private`	`sites/default/files/civicrm`
`civicrm.log`	`sites/default/files/civicrm/ConfigAndLog`	`[civicrm.private]/ConfigAndLog`
`civicrm.compile`	`sites/default/files/civicrm/templates_c`	`[civicrm.private]/templates_c`

General Effect: Before

In civicrm.settings.php, one sets CIVICRM_TEMPLATE_COMPILEDIR to be some value ending with templates_c, e.g.

define('CIVICRM_TEMPLATE_COMPILEDIR',  '/var/www/example.com/htdocs/sites/default/files/civicrm/templates_c');

Changing this field has surprising side-effects on other paths (which do not involve template-compilation) -- such as the public data-folders ([civicrm.files]) and the log folder (ConfigAndLog). Some effects can be mitigated by overriding $civicrm_paths['civicrm.files'], but not all.

General Effect: After

In civicrm.settings.php, one may set CIVICRM_TEMPLATE_COMPILEDIR as before. For good or ill, this will have the same surprising side-effects as before.

Alternatively, one may remove CIVICRM_TEMPLATE_COMPILEDIR. Instead, set all paths using $civicrm_paths. We present a few examples -- the consequences should be more intuitive.

Example 1: This is a minimalist example in which all public and private data go to the same folder, sites/default/files/civicrm.

$civicrm_paths['civicrm.files']['path'] = '/srv/example.com/htdocs/sites/default/files/civicrm';
$civicrm_paths['civicrm.private']['path'] = '/srv/example.com/htdocs/sites/default/files/civicrm';

The constant CIVICRM_TEMPLATE_COMPILEDIR is never consulted. Updating civicrm.files changes only public data-folders. Updating civicrm.private changes only private data-folders (e.g. templates_c and ConfigAndLogDir).

Example 2: The public and private data are stored in separate places, akin to Drupal's public and private folders.

$civicrm_paths['civicrm.files']['path'] = '/srv/example.com/htdocs/sites/default/files/civicrm');
$civicrm_paths['civicrm.private']['path'] = '/srv/example.com/private/civicrm');

Example 3: This is a maximalist example in which we reject most of the default paths and instead try to follow Linux FHS.

$civicrm_paths['civicrm.files']['path'] = '/var/www/sites/default/files/civicrm');
$civicrm_paths['civicrm.private']['path'] = '/var/lib/civicrm');
$civicrm_paths['civicrm.log']['path'] = '/var/log/civicrm');
$civicrm_paths['civicrm.compile']['path'] = '/var/run/civicrm-compile');
$civicrm_paths['civicrm.root']['path'] = '/usr/share/php/civicrm-core');
$civicrm_paths['civicrm.root']['url'] = 'https://example.com/civicrm-core'); // with httpd path alias
$civicrm_paths['cms.root']['path'] = '/usr/share/php/drupal-core');
$civicrm_paths['cms.root']['url'] = 'https://example.com/'); // with httpd path alias

Mechanics: Before

The templateCompileDir is calculated procedurally by CRM_Core_Config_Runtime using CIVICRM_TEMPLATE_COMPILEDIR.
The configAndLogDir is calculated procedurally by CRM_Core_Config_Runtime using CRM_Utils_File::baseFilePath() . '/configAndLog' (which in turn is derived from CIVICRM_TEMPLATE_COMPILEDIR).
There are several references to CIVICRM_TEMPLATE_COMPILEDIR in the codebase.

Mechanics: After

The templateCompileDir is calculated on-demand by Civi::paths(); it will be the first available item from this list:
1. $civicrm_paths['civicrm.compile']['path']
2. CIVICRM_TEMPLATE_COMPILEDIR
3. $civicrm_paths['civicrm.private']['path'] . '/templates_c'
4. ~~CRM_Utils_File::baseFilePath() . '/templates_c'~~ (This is a hypothetical scenario which roughly equates to rule "ii" above. It would be more elegant than rule "ii"; however, in some wonky configurations, rule "ii" provides better backward-compatibility. The presence of rule "ii" makes this code-path irrelevant.)
The configAndLogDir is calculated on-demand by Civi::paths(); it will be the first available item from this list:
1. $civicrm_paths['civicrm.log']['path']
2. $civicrm_paths['civicrm.private']['path'] . '/ConfigAndLog'
3. CRM_Utils_File::baseFilePath() . '/ConfigAndLog' (which in turn is derived from CIVICRM_TEMPLATE_COMPILEDIR)
There are are only two references to CIVICRM_TEMPLATE_COMPILEDIR.

Comments

This PR only changes the mechanism for determining paths - updates to configuration is left as a separate issue. In the phone meeting circa July 10, we decided to continue using CIVICRM_TEMPLATE_COMPILEDIR in the old installer (ie install/, civicrm.settings.php.template) and start using the new conventions in newer installers (ie civicrm-setup).

Original issue: https://lab.civicrm.org/dev/cloud-native/issues/3

civibot · 2019-07-03T05:41:42Z

(Standard links)

If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

totten · 2019-07-03T06:42:44Z

jenkins, test this please

herbdool · 2019-07-04T11:48:17Z

This looks like a good solution to the problem. I bet it's okay to go with the long term solution since people can still override them in the settings file if they have different locations for each. I will hopefully test today.

herbdool · 2019-07-04T16:09:34Z

CRM/Core/Config/Runtime.php

@@ -99,16 +92,6 @@ public function initialize($loadFromDB = TRUE) {
      $this->fatal('You need to define CIVICRM_TEMPLATE_COMPILEDIR in civicrm.settings.php');


We can probably get rid of this too. \CRM_Utils_File::baseFilePath() already has a fatal error check.

Good point. I'll update that.

herbdool · 2019-07-04T16:41:48Z

Civi/Core/Paths.php

+      })
+      ->register('civicrm.compile', function () {
+        return [
+          'path' => \Civi::paths()->getPath('[civicrm.private]/templates_c'),


Is this a problem for $file = CIVICRM_TEMPLATE_COMPILEDIR . "/CachedCiviContainer.{$envId}.php"; in \Civi\Core\Container::loadContainer()? Can that file be in a private directory? I'm guessing

Same with CRM_Extension_ClassLoader::getCacheFile() there's this line:
$file = CIVICRM_TEMPLATE_COMPILEDIR . "/CachedExtLoader.{$envId}.php";

Along with this maybe the only place that references CIVICRM_TEMPLATE_COMPILEDIR should be \CRM_Utils_File::baseFilePath so that people can't set the template directory twice. The other references could call [civicrm.private]/templates_c?

Oops. I guess \Civi\Core\Container::loadContainer() can't expect Civi\Core\Paths to be available. So CIVICRM_TEMPLATE_COMPILEDIR needs to stick around at least for this.

On Pantheon I've been setting CIVICRM_TEMPLATE_COMPILEDIR to the tmp directory to deal with a special issue there, and that works so long as [civicrm.files] is also set to point to the regular file system.

This is how civicrm.settings.php would be set up to work on Pantheon:

if (!defined('CIVICRM_TEMPLATE_COMPILEDIR')) { define( 'CIVICRM_TEMPLATE_COMPILEDIR', '/tmp/civicrm/templates_c/'); } $civicrm_paths['civicrm.compile']['path'] = CIVICRM_TEMPLATE_COMPILEDIR; $civicrm_paths['civicrm.files']['path'] = '/app/sites/default/files/civicrm/'; $civicrm_paths['civicrm.private']['path'] = '/app/sites/default/files/private/civicrm/';

For now might not need [civicrm.compile] since it's not being called anywhere, I think.

So maybe the only thing I'd borrow from #14719 is to make this one:

->register('civicrm.compile', function () { return [ 'path' => CIVICRM_TEMPLATE_COMPILEDIR, ]; })

The other paths are probably fine in this PR.

I think most remaining references to CIVICRM_TEMPLATE_COMPILEDIR come late enough during bootstrap that Civi::paths()->getPath() is viable substitute... but I'm gonna try it with some funny-cases (like GenCode and install/index.php) to be sure.

@herbdool So, going along the lines of your first comment, I've pushed an updated branch which patches \Civi\Core\Container::loadContainer(), CRM_Extension_ClassLoader, and CRM_Core_IDS to replace CIVICRM_TEMPLATE_COMPILEDIR with Civi::paths()->getFoo().

This brings it down to only one reference to CIVICRM_TEMPLATE_COMPILEDIR (in CRM_Utils_File::baseFilePath()).

I haven't tested the following claim, but it should mean that it is valid to completely omit CIVICRM_TEMPLATE_COMPILEDIR and instead specify civicrm.files and civicrm.private. By setting those two variables, one bypasses any remaining code-paths that rely directly on baseFilePath() or CIVICRM_TEMPLATE_COMPILEDIR.

Along the lines of your last comment, I updated the civicrm.compile calculation so that it consults CIVICRM_TEMPLATE_COMPILEDIR. Formally, the effective value of civicrm.compile will be the first defined item from this list:

$civicrm_paths['civicrm.compile']['path']

CIVICRM_TEMPLATE_COMPILEDIR

Civi::paths()->getPath('[civicrm.private]/templates_c')

I think this means that we get backward-compatibility for existing deployments as well as supporting a sensible style for new deployments:

(Backwards compatible style) If you have an existing deployment and existing civicrm.settings.php (with define('CIVICRM_TEMPLATE_COMPILEDIR'...) and no customization to $civicrm_paths, etc) - then CIVICRM_TEMPLATE_COMPILEDIR is still used.

(New style) If you make a new/revised deployment with a new/revised civicrm.settings.php (omitting define('CIVICRM_TEMPLATE_COMPILEDIR', ...); instead, using $civicrm_paths), then:

The default value of civicrm.compile is [civicrm.private]/templates_c

The default value of civicrm.log is [civicrm.private]/ConfigAndLog

Either can be customized directly (without impacting the parent).

totten · 2019-07-10T07:22:51Z

While working on this, I've submitted a few smaller/complementary PRs: #14777, #14778, #14770.

…perties Overview -------- This corrects a subtle bug in which reading/writing some properties in `CRM_Core_Config_MagicMerge`, which became visible in the tests for PR civicrm#14718. Before ------ * If a property has a simple name (e.g. `maxFileSize`), then `$cache`-reads and `$cache`-writes are consistent. * If a property has a split name (e.g. `maxAttachments`, `max_attachments`), then `$cache`-reads and `$cache`-writes are *not* consistent. Writes basically don't work. After ----- * The typical behavior (which applies to simple names) also applies to split names. Technical Details ----------------- To understand the change, you should skim `MagicMerge` and observe a few things: * In `getPropertyMap()`, observe that: * (a) Most properties have a singular name -- for example, `maxFileSize` has one name, which implies that it's the same name in the high-level facade (`$config->maxFileSize`) and in the underlying data-store (`settings->get('maxFileSize')`). * (b) Some properties are aliased. Ex: `$config->maxAttachments` corresponds to underlying item `settings->get('max_attachments')`. * In `__get()` and `__set()`, note that properties are loaded initally from the underlying data-store once; thereafter, any reads or writes go to `$this->cache`. That's a thread-local place that stores temporary revisions. To see the cache consistency problem, specifically note that: * `__get()` consistently references `$this->cache[$k]` * `__set()` was confused; at the start, it used `$this->cache[$k]`, and later it used `$this->cache[$name]`. This PR adds a unit-test which ensures that the thread-local/cached value works consistently.

…perties Overview -------- `CRM_Core_Config_MagicMerge` allows properties to be temporarily modified (for the duration the object's lifespan). However, this behavior does not work correctly if the property-name uses aliasing. The PR addresses a test-failure that became visible in civicrm#14718, and it adds test-coverage for some typical and atypical examples. Before ------ * (Vast majority of properties) If a property has a simple name (e.g. `maxFileSize`), then `$cache`-reads and `$cache`-writes are consistent. * (Handful of properties) If a property has a split name (e.g. `maxAttachments`, `max_attachments`), then `$cache`-reads and `$cache`-writes are *not* consistent. Writes basically don't work. After ----- * Aliased properties work the same as normal/non-aliased. Technical Details ----------------- To understand the change, you should skim `MagicMerge` and observe a few things: * In `getPropertyMap()`, observe that: * (a) Most properties have a singular name -- for example, `maxFileSize` has one name, which will be the same in the high-level facade (`$config->maxFileSize`) and in the underlying data-store (`settings->get('maxFileSize')`). * (b) Some properties are aliased. Ex: `$config->maxAttachments` corresponds to underlying item `settings->get('max_attachments')`. * In `__get()` and `__set()`, note that properties are loaded initally from the underlying data-store once; thereafter, any reads or writes go to `$this->cache`. That's a thread-local place that stores temporary revisions. To see the cache consistency problem, specifically note that: * `__get()` consistently references `$this->cache[$k]` * `__set()` was confused; at the start, it used `$this->cache[$k]`, and later it used `$this->cache[$name]`. This PR adds a unit-test which ensures that the thread-local/cached value works consistently.

totten · 2019-07-12T23:25:19Z

The reported test-failure arises because of a latent bug. Specifically, CRM_Core_ErrorTest relies on an ephemeral override for $config->configAndLogDir. Ephemeral overrides work most of the time, but not all of the time, and this change walked into an edge-case where ephemeral overrides don't work. Proposed fix: #14818

herbdool · 2019-07-14T15:06:31Z

I guess this needs an update from master before bothering to run tests again

…rom Civi::paths

…Runtime" to "Paths". Use hierarchy. This slightly changes the computation of the private log and compilation directories so that: * The definition appears more declarative. * The default is computed on-demand (i.e. later in the process). * The values can be overriden more intuitively. Before ------ The properties `$config->configAndLogDir` and `$config->templateCompileDir` are computed as part of a procedural initialization step in `CRM_Core_Config_Runtime`. After ----- The properties `$config->configAndLogDir` and `$config->templateCompileDir` are computed on-demand by `Civi\Core\Paths`. The values of these properties may: * Be computed automatically in a way that matches the previous output (i.e. `configAndLogDir`=~`CIVICRM_TEMPLATE_COMPILEDIR/../configAndLog`; `templateCompileDir`=~`CIVICRM_TEMPLATE_COMPILEDIR`). * Be customized as a pair (by setting `civicrm.private`) * Be customized individually (by setting `civicrm.logs` or `civicrm.compile`).

1. Previous commits generally removed any responsibility for `CIVICRM_TEMPLATE_COMPILEDIR` from `Runtime.php`, so the validation is longer local/relevant. 2. The `CIVICRM_TEMPLATE_COMPILEDIR` is still used in `CRM_Utils_File::baseFilePath()` which has a similar validation step.

…R to 'civicrm.compile' The broader PR seeks to make path computation more intuitive, which requires computing the path to `templates_c` using a function. This PR replaces the reference to `CIVICRM_TEMPLATE_COMPILEDIR` with a function-call to `Civi::paths()->getVariable()`. Is it safe change to make this change in `CRM_Core_IDS`? Will the `Civi::paths()` be callable? Yes, here's why I believe so: * (Big picture) Civi completes the bootstrap process (`CRM_Core_Config::singleton()`) and brings critical services online *before* the invoker does any routing or page-processing. The invoker (`CRM_Core_Invoke::runItem()`) is the only thing which uses `CRM_Core_IDS`. * (Little picture) In the changed function `CRM_Core_IDS::createBaseConfig()`, observe that it calls `CRM_Core_Config::singleton()` before running this modified code. That's a tell-tale sign of code that already relies on having a booted system.

…M_TEMPLATE_COMPILEDIR to 'civicrm.compile' The broader PR seeks to make path computation more intuitive, which requires computing the path to `templates_c` using a function. This PR replaces the reference to `CIVICRM_TEMPLATE_COMPILEDIR` with a function-call to `Civi::paths()->getPath()`. Why change these two files in the same commit? Because they're basically doing the same thing (writing an executable PHP file to the template cache), and the demonstration of their safety is basically the same. Is it safe to change this reference to `CIVICRM_TEMPLATE_COMPILEDIR` in `Civi\Core\Container::loadContainer()` and `CRM_Extension_ClassLoader::register()/::getCacheFile()`? Yes, I believe so: * Look at `Civi\Core\Container::boot()`. * Observe that it initializes services in two general stages: * First, the `$bootServices` (`runtime`, `paths`, `userSystem`, etc) * Second, the DB/extension-dependent services (`CRM_Utils_Hook`, `CRM_Extension_System`, `loadContainer()`). * The first stage services (e.g. `paths`) provide enough information to process `Civi::paths()->getPath('[civicrm.compile]/foo')`. * All the modified lines run as part of the *second* stage (i.e. after the `$bootServices` have been initialized).

totten · 2019-07-16T04:36:45Z

@herbdool yup, I've rebased again for current master and fleshed out the PR description.

totten · 2019-07-16T06:50:25Z

(NOTE: Edited to use more concrete examples of civicrm.settings.php data.)

So, doing a bit of self-review on this, I've been thinking about what the final result looks like to a sysadmin who wants to take control over all the paths -- and the PR aggravates a discrepancy in the overall admin experience. Compare:

$civicrm_paths['civicrm.compile']['path'] = '/var/www/files/civicrm/templates_c';
$civicrm_paths['civicrm.log']['path'] = '/var/www/files/civicrm/ConfigAndLog';
$civicrm_settings['domain']['imageUploadDir'] = '/var/www/files/civicrm/persist/contribute';
$civicrm_settings['domain']['extensionsDir'] = '/var/www/files/civicrm/ext';

From an admin perspective, there's very little conceptual difference between "log dir" and "image dir". OTOH, it didn't initially occur to me that configAndLogDir and templateCompileDir should be moved into the settings system like imageUploadDir and extensionsDir. That's partially a bias (i.e. if I had my druthers, none of these things would appear in UI - they'd all be config-file/PHP-only), and it's partially a technical assumption (that the data must be available during early bootstrap).

But as I reason through it, I don't think the discrepancy is a technical necessity -- one could potentially reconcile by either:

Prefer Civi::paths() for path/URL-related configuration. After this PR, also update the handling of imageUploadDir, extensionsDir, etc so that they use Civi::paths(). Slowly deprecate the use of web UI/settings for managing paths. A customized civicrm.settings.php might look like:

$civicrm_paths['civicrm.compile']['path'] = '/var/www/files/civicrm/templates_c';
$civicrm_paths['civicrm.log']['path'] = '/var/www/files/civicrm/ConfigAndLog';
$civicrm_paths['civicrm.image']['path'] = '/var/www/files/civicrm/persist/contribute';
$civicrm_paths['civicrm.ext']['path'] = '/var/www/files/civicrm/ext';

Prefer Civi::settings() for path/URL-related configuration. Change this PR so that the handling of private folders better parallels the handling of public folders.
- [civicrm.private] would work like [civicrm.files] (i.e. using Civi::paths())
- templateCompileDir would work like extensionsDir (i.e. using Civi::settings(), evaluated with Civi::paths()). A customized civicrm.settings.php might look like:
```
$civicrm_settings['domain']['templateCompileDir'] = '[civicrm.private]/templates_c';
$civicrm_settings['domain']['configAndLogDir'] = '[civicrm.private]/ConfigAndLog';
$civicrm_settings['domain']['imageUploadDir'] = '[civicrm.files]/persist/contribute';
$civicrm_settings['domain']['extensionsDir'] = '[civicrm.files]/ext';
```

Moving these things out of the DB settings and strictly into config/PHP files feels like a cleaner end-game, but it's going to look like a bigger change.

Preferring Civi::settings() feels a bit riskier on a technical level, but (for somebody following along with the system) it probably seems like the more consistent/familiar change.

eileenmcnaughton · 2019-07-22T00:10:00Z

@herbdool have the issues raised / discussed been resolved now?

herbdool · 2019-07-24T21:13:04Z

@eileenmcnaughton the issues I had have been resolved. Looks like regressions have been fixed, tests passing.

As for @totten's questions about approach, I'm leaning towards deprecating using web/UI for setting paths. Perhaps it's possible to first use Civi::paths behind the scenes and have it populate/override paths set in $civicrm_settings? I already usually try to set all the paths in civicrm.settings.php since it's such a pain when we have to create dev environments if we don't.

eileenmcnaughton · 2019-07-24T22:04:21Z

@totten does anything in @herbdool's comments above delay me from merging this? I feel happy with @herbdool as reviewer here

eileenmcnaughton · 2019-07-24T22:34:45Z

I'm going to merge this & this create a documentation issue to follow up & maybe follow through a bit

eileenmcnaughton · 2019-07-24T22:37:09Z

Place for follow up discussion https://github.com/civicrm/civicrm-dev-docs/issues/651

civibot bot added the master label Jul 3, 2019

totten mentioned this pull request Jul 3, 2019

Lookup path-variable for the private filepath #12843

Closed

herbdool suggested changes Jul 4, 2019

View reviewed changes

totten force-pushed the herb-private-hier branch 2 times, most recently from 138b4e1 to a1eb80a Compare July 10, 2019 07:08

totten mentioned this pull request Jul 10, 2019

(WIP) Move configAndLogDir/templateCompileDir from "Runtime" to "Paths" (compat variant) #14719

Closed

MegaphoneJon added the needs-documentation label Jul 11, 2019

totten mentioned this pull request Jul 12, 2019

MagicMerge - Fix ephemeral overrides for aliased properties #14818

Merged

herbdool and others added 9 commits July 15, 2019 18:58

Lookup path-variable for the private filepath

86a36a7

Boot paths after runtime

b1db062

Use global since Civi\Core\Paths isn't loaded yet

b90c445

Specify path

6c5f4a4

(dev/cloud-native#3) MagicMerge - Allow pulling properties directly f…

00f6fb2

…rom Civi::paths

totten force-pushed the herb-private-hier branch from a1eb80a to 6f50d29 Compare July 16, 2019 02:34

totten changed the title ~~(WIP) Move configAndLogDir/templateCompileDir from "Runtime" to "Paths" (hierarchical variant)~~ Move log and compilation dirs from "Runtime" to "Paths" Jul 16, 2019

eileenmcnaughton merged commit 511a740 into civicrm:master Jul 24, 2019

herbdool mentioned this pull request Jul 25, 2019

Remove patch to allow custom private paths herbdool/civicrm-starterkit-drops-7#16

Closed

totten deleted the herb-private-hier branch July 30, 2019 22:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move log and compilation dirs from "Runtime" to "Paths" #14718

Move log and compilation dirs from "Runtime" to "Paths" #14718

totten commented Jul 3, 2019 •

edited

Loading

civibot bot commented Jul 3, 2019

totten commented Jul 3, 2019

herbdool commented Jul 4, 2019

herbdool Jul 4, 2019

totten Jul 10, 2019

herbdool Jul 4, 2019 •

edited

Loading

herbdool Jul 4, 2019

herbdool Jul 4, 2019

herbdool Jul 4, 2019

totten Jul 10, 2019

totten Jul 10, 2019

totten Jul 10, 2019

totten commented Jul 10, 2019

totten commented Jul 12, 2019

herbdool commented Jul 14, 2019

totten commented Jul 16, 2019

totten commented Jul 16, 2019 •

edited

Loading

eileenmcnaughton commented Jul 22, 2019

herbdool commented Jul 24, 2019

eileenmcnaughton commented Jul 24, 2019

eileenmcnaughton commented Jul 24, 2019

eileenmcnaughton commented Jul 24, 2019

		@@ -99,16 +92,6 @@ public function initialize($loadFromDB = TRUE) {
		$this->fatal('You need to define CIVICRM_TEMPLATE_COMPILEDIR in civicrm.settings.php');

Move log and compilation dirs from "Runtime" to "Paths" #14718

Move log and compilation dirs from "Runtime" to "Paths" #14718

Conversation

totten commented Jul 3, 2019 • edited Loading

Overview

General Effect: Before

General Effect: After

Mechanics: Before

Mechanics: After

Comments

civibot bot commented Jul 3, 2019

totten commented Jul 3, 2019

herbdool commented Jul 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

herbdool Jul 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

totten commented Jul 10, 2019

totten commented Jul 12, 2019

herbdool commented Jul 14, 2019

totten commented Jul 16, 2019

totten commented Jul 16, 2019 • edited Loading

eileenmcnaughton commented Jul 22, 2019

herbdool commented Jul 24, 2019

eileenmcnaughton commented Jul 24, 2019

eileenmcnaughton commented Jul 24, 2019

eileenmcnaughton commented Jul 24, 2019

totten commented Jul 3, 2019 •

edited

Loading

herbdool Jul 4, 2019 •

edited

Loading

totten commented Jul 16, 2019 •

edited

Loading