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

Require Zulip Server 5 #268

Open
gnprice opened this issue Aug 14, 2023 · 2 comments
Open

Require Zulip Server 5 #268

gnprice opened this issue Aug 14, 2023 · 2 comments
Labels
a-api Implementing specific parts of the Zulip server API

Comments

@gnprice
Copy link
Member

gnprice commented Aug 14, 2023

Currently the app works with any Zulip Server version 4.x or newer. This is documented in our README and reflects what's documented at https://zulip.readthedocs.io/en/latest/overview/release-lifecycle.html . With #267, we'll start giving an explicit error if the user tries to connect to an older server.

At some point we'll want to increase that minimum to 5.x. As recorded at https://blog.zulip.com/tag/major-releases/ , 4.0 was released in 2021-05, over 2 years ago, and 5.0 in 2022-03, approaching 1.5 years ago.

When we do, we can follow up by sweeping the codebase for TODO(server-5) comments and acting on all of those. That will be in the same spirit as zulip/zulip-mobile#5100 and zulip/zulip-mobile#5192 in the old zulip-mobile RN app. (And zulip/zulip-mobile#5708, though that's a draft that was made moot by our migration to Flutter.)


There is one spot where we already don't fully support older than server 5.x: currently we ignore user settings on such servers, falling back to defaults. As I wrote at #261 (comment) (reviewing @chrisbobbe's PR which introduced support for user settings and skipped it for those old servers):

Cool, sounds good. This does seem like a spot where properly supporting both old and new versions would be kind of a pain. (Would have been totally doable, if the timing were such that we had to — we'd probably use a readValue that would just be one more spot that needs another line or two of code per setting — but enough of a pain to not be worth it given that we don't have to.)

The assumption that relies on is that we will desupport server 4.x (or if not then go back and deal with the old settings API after all) before launch.

@gnprice gnprice added the a-api Implementing specific parts of the Zulip server API label Aug 14, 2023
@gnprice gnprice added this to the Beta milestone Aug 14, 2023
@gnprice
Copy link
Member Author

gnprice commented Aug 14, 2023

There is one spot where we already don't fully support older than server 5.x

(But in general we should avoid introducing more such places for now. As I wrote at #238 (comment) :

We'll probably bump that version higher sometime soon. But until we do that explicitly, I'd like to keep the code uniformly supporting the version we say we're supporting. That helps keep it manageable to be able to look at a piece of code and then at the API docs and confirm the code is behaving as it should, and to avoid accidentally leaving out something we need on newer versions.

)

@gnprice
Copy link
Member Author

gnprice commented Jan 29, 2024

When we do this, we should probably express the check in terms of a Zulip feature level, rather than a Zulip Server version in a form like "5.0". See chat discussion.

OTOH when the check fails and we're giving an error message to the user, that explanation should probably say the minimum is "Zulip Server 5.0". So for example:

Could not connect

The Zulip server at https://chat.example.com/ is running an unsupported old version of Zulip.

The minimum supported version is Zulip Server 5.0 (feature level 122).

The server at https://chat.example.com/ is at version 4.0-dev-g012345678 and feature level 37.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-api Implementing specific parts of the Zulip server API
Projects
Status: No status
Development

No branches or pull requests

1 participant