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.

Sign up for GitHub

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

Jump to bottom

fix: remove mention of disabling file locking from config sample #45330

Merged
merged 1 commit into from
Nov 15, 2024
Merged

fix: remove mention of disabling file locking from config sample #45330

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 0 additions & 15 deletions config/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2269,21 +2269,6 @@
*/
'max_filesize_animated_gifs_public_sharing' => 10,


Copy link
Member

Choose a reason for hiding this comment

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

GitHub won't let me make suggestions on deleted lines, but what if instead of removing it we adjusted the language to be a bit more scary like the documentation already is + tried to emphasize it's not the same as files_lock:

/**
 * Transactional file locking
 * Locks files to avoid file corruption during normal operation. 
 * 
 * WARNING: Transactional file locking is not for preventing multiple users from editing 
 * the same document, or giving notice that other users are working on the same document. 
 * Multiple users can open and edit a file at the same time and Transactional File 
 * locking does not prevent this. Rather, it prevents simultaneous file saving. This
 * is enabled by default and should generally be left enabled. 
 * 
 * IMPORTANT: If you are experiencing problems with user facing locking, such 
 * as to avoid simultaneously edits, you may be looking for the `files_lock` app and 
 * its settings and/or settings within the client apps you're using. Do not disable this
 * parameter unless you are extremely sure you know what you're doing.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would rather also remove the mention of the option from the documentation. I can't think of a single case where disabling it would be advisable.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. 👍

The toggle was only added for turning it on from the looks of it (when it wasn't on by default since it was still experimental at the time): 2f4f468

That need no longer applies today.

Made sense to probably keep it around for a bit when it switched to being on by default in 4880d77, but those days are long over. :-)

/**
* Enables transactional file locking.
* This is enabled by default.
*
* Prevents concurrent processes from accessing the same files
* at the same time. Can help prevent side effects that would
* be caused by concurrent operations. Mainly relevant for
* very large installations with many users working with
* shared files.
*
* Defaults to ``true``
*/
'filelocking.enabled' => true,

/**
* Set the lock's time-to-live in seconds.
*
Expand Down
Loading