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.

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

feat: Sync Config::DeleteServerAfter across devices #5753

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

iequidoo
Copy link
Collaborator

@iequidoo iequidoo commented Jul 5, 2024

No description provided.

@iequidoo iequidoo marked this pull request as ready for review July 5, 2024 16:43
@iequidoo iequidoo requested a review from r10s July 5, 2024 16:43
@iequidoo
Copy link
Collaborator Author

iequidoo commented Aug 2, 2024

Btw too small DeleteServerAfter values can break the synchronisation itself because other devices have little chance to see sync messages. Maybe we should add a lower limit on synchronised values?

@iequidoo iequidoo requested a review from link2xt August 2, 2024 02:15
@iequidoo iequidoo marked this pull request as draft August 12, 2024 20:11
@iequidoo iequidoo force-pushed the iequidoo/sync-DeleteServerAfter branch from c0601ca to 2999df4 Compare August 13, 2024 03:16
@iequidoo
Copy link
Collaborator Author

Btw too small DeleteServerAfter values can break the synchronisation itself because other devices have little chance to see sync messages. Maybe we should add a lower limit on synchronised values?

Maybe this is not critical because DeleteServerAfter can be increased on all devices reliably because the setting is applied before the decision of sync message removal is made. At least it looks working, added a Python test for this.

@iequidoo iequidoo force-pushed the iequidoo/sync-DeleteServerAfter branch from 2999df4 to bb421f4 Compare August 13, 2024 16:45
@iequidoo iequidoo marked this pull request as ready for review August 13, 2024 17:04
@Hocuri
Copy link
Collaborator

Hocuri commented Oct 22, 2024

Was there any discussion about this?

The downside of syncing DeleteServerAfter is of course that the following use case doesn't work:

  • Have a PC, which is running at least once a week, and a phone, which is running constantly (more than once a day)
  • On the phone, set DeleteServerAfter to 1 week, so that the PC has a chance to download the messages. On the PC, set DeleteServerAfter to 1 day, so that messages are quickly deleted.

OTOH, this is a very advanced use case, and most users won't understand how to configure this.

@iequidoo
Copy link
Collaborator Author

Was there any discussion about this?

I've already forgotten who suggested (to think about?) this, but probably @r10s as i added them to reviewers :)

The downside of syncing DeleteServerAfter is of course that the following use case doesn't work:

  • Have a PC, which is running at least once a week, and a phone, which is running constantly (more than once a day)

  • On the phone, set DeleteServerAfter to 1 week, so that the PC has a chance to download the messages. On the PC, set DeleteServerAfter to 1 day, so that messages are quickly deleted.

This is still possible to configure as for any other synchronised setting: turn BccSelf off, change settings so that no sync messages are sent, turn on BccSelf back. Rather there's a question which scenario is more often -- having DeleteServerAfter different or the same on different devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants