-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
gui, lib/versioner: Allow to configure staggered versioning intervals (fixes #8352) #9094
base: main
Are you sure you want to change the base?
gui, lib/versioner: Allow to configure staggered versioning intervals (fixes #8352) #9094
Conversation
…fixes syncthing#8352) Currently, there are four intervals used in the staggered versioning, with all of them but the Maximum Age being hard-coded. With this change, the user is able to configure all intervals to their liking. Due to the complexity of the configuration, this option is intended to be used by advanced users only. In addition to the above, also: 1. Add yet another interval for more fine-grained configuration possibilities. By default, the new interval activates after one year, keeping one version per month. However, because the default Maximum Age is also set to one year, this fifth interval is actually not utilised unless the user increases the Maximum Age to a higher value. 2. Fix a small bug in the Staggered Versioning test where the default time period for the third interval was supposed to be set to 30 days, but in reality it was set to just 7 days. Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
I will be grateful for any tips on how to simplify this code 😅. |
@@ -115,19 +115,89 @@ | |||
<span translate ng-if="folderEditor.simpleKeep.$error.min && folderEditor.simpleKeep.$dirty">You must keep at least one version.</span> | |||
</p> | |||
</div> | |||
<div class="form-group" ng-if="currentFolder._guiVersioning.selector=='staggered'" ng-class="{'has-error': folderEditor.staggeredMaxAge.$invalid && folderEditor.staggeredMaxAge.$dirty}"> | |||
<p class="help-block"><span translate>Files are moved to date stamped versions in a .stversions directory when replaced or deleted by Syncthing.</span> <span translate>Versions are automatically deleted if they are older than the maximum age or exceed the number of files allowed in an interval.</span></p> | |||
<p translate class="help-block">The following intervals are used: for the first hour a version is kept every 30 seconds, for the first day a version is kept every hour, for the first 30 days a version is kept every day, until the maximum age a version is kept every week.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to break this very long translation, so unfortunately parts of it will have to be translated again, but this time I've also tried to split this into separate strings, so that it should be easier to deal with from now on if there are any small changes required.
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Signed-off-by: Tomasz Wilczyński <twilczynski@naver.com>
Didn't look at any code yet, but I can say that exposing config values in the range of millions of seconds in the non-advanced GUI certainly doesn't seem ideal. Humans generally use different units at those time scales. |
Yeah, I know about that. I originally just tried using time scales that are mentioned in the code comments (day/hour/week), however that didn't allow for such fine-grained control as the current iteration (which the user in the enhancement request wanted). I think there could be two solutions to this:
or
The second approach would probably be ideal and much more flexible, but I'm not sure if I can pull this off in a reasonable amount of time without extreme code complexity, so for now I'll probably just try out the first one and see how it works. Edit: I'm in the process of re-writing this. The final code is going to be much different than the current iteration, so if anyone is willing to look at this PR, please wait for the next version and don't waste your time on what is available now 😅. I will make it clear when the PR is ready for review. |
The information about default intervals is helpful! |
The information is already there right now! It's just because I added one more interval, I had to modify the sentence, and I though that the whole thing would be easier to read and understand when split into a bullet point list.
Do you think it would be better to have it displayed in a separate modal? It's definitely possible to do, but this part is only visible when you're using Staggered Versioning already, so that's why I just added it to the main File Versioning configuration window together with the other, already existing options. Of course, it's not visible when using other types of versioning (or no versioning at all). Using human language would be nice, but this kind of sentence is a nightmare when it comes to translation. I don't think it's actually possible (e.g. because "2 minutes" and "5 minutes" use different noun forms depending on the number in languages like Polish).
This has recently been changed in #8987. We now show all values regardless of whether they are default or not. |
gui, lib/versioner: Allow to configure staggered versioning intervals (fixes #8352)
Currently, there are four intervals used in the staggered versioning,
with all of them but the Maximum Age being hard-coded. With this change,
the user is able to configure all intervals to their liking. Due to the
complexity of the configuration, this option is intended to be used by
advanced users only.
In addition to the above, also:
Add yet another interval for more fine-grained configuration
possibilities. By default, the new interval activates after one year,
keeping one version per month. However, because the default Maximum
Age is also set to one year, this fifth interval is actually not
utilised unless the user increases the Maximum Age to a higher value.
Fix a small bug in the Staggered Versioning test where the default
time period for the third interval was supposed to be set to 30 days,
but in reality it was set to just 7 days.
Signed-off-by: Tomasz Wilczyński twilczynski@naver.com
Screenshots