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

Remove experimental zeroskip database backend #5199

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

Conversation

rsto
Copy link
Member

@rsto rsto commented Jan 8, 2025

It never was ready for production use, is not distributed anymore in cyruslibs and is not developed further.

@rsto rsto requested review from rjbs, ksmurchison, elliefm and brong January 8, 2025 13:04
cunit/aaa-db.testc Show resolved Hide resolved
cunit/libconfig.testc Outdated Show resolved Hide resolved
lib/imapoptions Outdated
@@ -217,7 +217,7 @@ are listed with ``<none>''.
/* Alternative INBOX spellings that can't be accessed in altnamespace
otherwise go under here. */

{ "annotation_db", "twoskip", STRINGLIST("skiplist", "twoskip", "zeroskip"), "3.1.6" }
{ "annotation_db", "twoskip", STRINGLIST("skiplist", "twoskip"), "3.1.6" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to replace the version number with "UNRELEASED". This applies to all the other foo_db option changes too, except for backup_db which I'll address separately

Copy link
Member Author

Choose a reason for hiding this comment

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

done. please recheck.

lib/imapoptions Outdated
@@ -549,7 +549,7 @@ Blank lines and lines beginning with ``#'' are ignored.
{ "backup_retention", "7d", DURATION, "3.12.0", "3.12.0" }
/* Deprecated. No longer used. */

{ "backup_db", "twoskip", STRINGLIST("skiplist", "sql", "twoskip", "zeroskip"), "3.12.0", "3.12.0" }
{ "backup_db", "twoskip", STRINGLIST("skiplist", "sql", "twoskip"), "3.12.0", "3.12.0" }
Copy link
Contributor

Choose a reason for hiding this comment

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

This case is interesting, because the option is already deprecated (since 3.12.0), but we're changing it again (by necessity I think).

I think perhaps you should change the first version number (which is the last_modified field) to "UNRELEASED", to reflect that it has been modified; but leave the second version number (the deprecated_since field) as "3.12.0", since that was when it was deprecated and that fact hasn't changed.

If that doesn't work, then I guess leave both version numbers as "3.12.0".

Copy link
Member Author

Choose a reason for hiding this comment

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

done. please recheck.

It never was ready for production use, is not distributed
anymore in cyruslibs and is not developed further.
@rsto rsto force-pushed the cyr-1417-remove-zeroskip branch from ed44d60 to f9d140a Compare January 10, 2025 08:45
@rsto rsto requested a review from elliefm January 10, 2025 08:46
Copy link
Contributor

@ksmurchison ksmurchison left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants