PHPORM-174 Add doc for cache and locks #2909
Conversation
docs/cache.txt
Outdated
|
|
||
|
|
||
| .. code-block:: php | ||
| :emphasize-lines: 9,14 |
There was a problem hiding this comment.
I think you may have meant to highlight 9 and 13? The function names?
| :emphasize-lines: 9,14 | |
| :emphasize-lines: 9,13 |
There was a problem hiding this comment.
I think 14 is correct, it's the line with return $this->cache->increment('counter');
There was a problem hiding this comment.
You are right. Both lines need to be udpated.
| :emphasize-lines: 9,14 | |
| :emphasize-lines: 10,15 |
There was a problem hiding this comment.
I think these should be lines 10 and 15 to highlight the CacheManager line and the return this-> ... line
docs/cache.txt
Outdated
|
|
||
|
|
||
| .. code-block:: php | ||
| :emphasize-lines: 9,14 |
There was a problem hiding this comment.
I think 14 is correct, it's the line with return $this->cache->increment('counter');
Co-authored-by: Jordan Smith <[email protected]>
Co-authored-by: Jordan Smith <[email protected]>
| - **Required**. The database connection used to store cache items. It must be a ``mongodb`` connection. | ||
|
|
||
| * - ``collection`` | ||
| - Default ``cache``. Name of the MongoDB collection to store cache items. |
There was a problem hiding this comment.
I didn't pick up on this when reviewing the original PRs, so forgive me for asking this late: where does the database name come from? Are you relying on whatever is configured through the corresponding connection option (for both this and lock_collection)?
There was a problem hiding this comment.
The database name is set in the database connection configuration. Do you think we should add an option to change the database name for cache and lock?
There was a problem hiding this comment.
You can also consider allowing a namespace for the collection and lock_collection options, and parsing out a database name by exploding on a dot character (no issue since it's prohibited in each part per Naming Restrictions.
Otherwise, I'm fine keeping this as-is and revisiting if/when someone asks for further customization.
There was a problem hiding this comment.
Tracking this feature: PHPORM-176
| 'stores' => [ | ||
| 'mongodb' => [ | ||
| 'driver' => 'mongodb', | ||
| 'connection' => 'mongodb', |
There was a problem hiding this comment.
Should collection and various lock options be specified here?
Also, is there any significance to the line break between default and stores in the configuration array? I believe you're already emphasizing the default line, so I don't think a line break is necessary for readability.
There was a problem hiding this comment.
Here, I rely on the default values. Do you think I should make a note of that?
There was a problem hiding this comment.
Calling out your reliance on default values makes sense to me.
I was mostly concerned with collection. The MongoLock constructor requires a value for its Collection parameter, and I didn't see where the default gets supplied.
There was a problem hiding this comment.
"connection" is required, "collection" has a default value which is cache. These 2 words are so similar that they can easily be confused.
The default is applied in the service provider.
Co-authored-by: Jordan Smith <[email protected]>
Co-authored-by: Jordan Smith <[email protected]>
Fix PHPORM-174
Documentation for #2891 and #2877
Docs PR: https://github.com/10gen/docs-laravel/pull/76
Preview: https://preview-mongodbgromnan.gatsbyjs.io/laravel/PHPORM-174/cache/