[Lock] Default path for FlockStore #13042
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OskarStark
reviewed
Feb 4, 2020
OskarStark
approved these changes
Feb 4, 2020
HeahDude
reviewed
Feb 4, 2020
|
Thank you @Guikingone I extended the code comment before, rather then keeping it as a note. To me it is an implementation detail which should be mentioned somewhere but not as prominent as a note. I also addressed @HeahDude's comment. All done in 3ea0737 |
OskarStark
added a commit
that referenced
this pull request
Feb 5, 2020
This PR was merged into the 3.4 branch.
Discussion
----------
[Lock] Default path for FlockStore
Hi everyone 👋
As noticed in the source code:
```php
/**
* @param string|null $lockPath the directory to store the lock, defaults to the system's temporary directory
*
* @throws LockStorageException If the lock directory doesn’t exist or is not writable
*/
public function __construct(string $lockPath = null)
{
if (null === $lockPath) {
$lockPath = sys_get_temp_dir();
}
if (!is_dir($lockPath) || !is_writable($lockPath)) {
throw new InvalidArgumentException(sprintf('The directory "%s" is not writable.', $lockPath));
}
$this->lockPath = $lockPath;
}
```
`sys_get_temp_dir` is the default path is null is given in the constructor, could it be a good idea to notice this? 🤔
Commits
-------
efbb0b3 refacto(Lock): FlockStore default path
OskarStark
added a commit
that referenced
this pull request
Feb 5, 2020
* 3.4: minor. refs #13042 refacto(Lock): FlockStore default path
OskarStark
added a commit
that referenced
this pull request
Feb 5, 2020
* 4.4: minor. refs #13042 refacto(Lock): FlockStore default path
OskarStark
added a commit
that referenced
this pull request
Feb 5, 2020
* 5.0: minor. refs #13042 refacto(Lock): FlockStore default path
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi everyone 👋
As noticed in the source code:
sys_get_temp_diris the default path is null is given in the constructor, could it be a good idea to notice this? 🤔