Skip to content

Conversation

r10s
Copy link
Contributor

@r10s r10s commented Sep 1, 2025

3 months were proven to be too short some years ago, after that issue, we went far up to 12 months.
however, 12 months were considered too long after recent discussions :) so, 6 months seems to be a good compromise.

the warning is still repeated every months and the text is unchanged.

advantage is still that this approach does not require network or opt-in, and catches really all lazy updaters with few effort, cmp deltachat/deltachat-desktop#5422

3 months were proven to be too short some years ago,
after that issue, we went far up to 12 months.
however, 12 months were considered too long after recent discussions :)
so, 6 months seems to be a good compromise.

the warning is still repeated every months and the text is unchanged.
maybe_warn_on_outdated(
&t,
timestamp_now + 180 * 24 * 60 * 60,
timestamp_now + 90 * 24 * 60 * 60,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be DC_OUTDATED_WARNING_DAYS - 1 instead of 90? As to me, it looks correct to use constants in tests. This is a test on the warning logic, not on the constant value

Copy link
Contributor Author

@r10s r10s Sep 1, 2025

Choose a reason for hiding this comment

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

i see that also as a test for reasonable value

testing that 90 days are not outdated makes sense, as we had that as an issue in the past and we do not want a regression

but there could be a another test for DC_OUTDATED_WARNING_DAYS - 1

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not clear why 90 days is a reasonable value and, say, 10 isn't, w/o a comment describing the issue. Currently nothing protects a random contributor from decreasing both values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i created a PR adding a comment at #7155

@r10s r10s merged commit 6b338a9 into main Sep 1, 2025
29 checks passed
@r10s r10s deleted the r10s/outdated-after-183-days branch September 1, 2025 15:44
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.

4 participants