-
Notifications
You must be signed in to change notification settings - Fork 208
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
Rename "stream" within codebase to "channel" #631
Comments
Fixes parts of zulip#631
Fixes parts of zulip#631
This lets the Dart compiler see that these are definitely constant through the function (which it can't assume when they're getters on an object). That in turn lets it narrow the types based on our null checks, so that we can leave out `!` operators later. That's small now, but will be more useful as we add more logic to this function -- if we had to scatter `!` all around that logic, we'd have to wonder whether some of them were unjustified assumptions and might throw at runtime. Also rename one local to "newStreamId", matching the property it mirrors. We'll rename both the variable and the property together, as part of a sweep coming up soon (zulip#631).
This lets the Dart compiler see that these are definitely constant through the function (which it can't assume when they're getters on an object). That in turn lets it narrow the types based on our null checks, so that we can leave out `!` operators later. That's small now, but will be more useful as we add more logic to this function -- if we had to scatter `!` all around that logic, we'd have to wonder whether some of them were unjustified assumptions and might throw at runtime. Also rename one local to "newStreamId", matching the property it mirrors. We'll rename both the variable and the property together, as part of a sweep coming up soon (#631).
Fixes parts of zulip#631
Fixes parts of zulip#631
…`Channel` Fixes parts of zulip#631
Fixes parts of zulip#631
…`Channel` Fixes parts of zulip#631
Fixes parts of zulip#631
…`Channel` Fixes parts of zulip#631
Fixes parts of zulip#631
…lRecipient Fixes parts of zulip#631
Fixes parts of zulip#631
Fixes parts of zulip#631
…lRecipient Fixes parts of zulip#631
Fixes parts of zulip#631
Fixes parts of zulip#631
Fixes parts of zulip#631
Fixes parts of zulip#631
Fixes parts of zulip#631
Fixes parts of zulip#631
Thanks @Khader-1 for all your work renaming a large swath of our "stream" references! In order to focus on getting this new app ready to roll out to our users as soon as we can, I'm pushing back the priority of the remainder of this issue. For the interim, when writing new code, we'll use "channel" names whenever they don't feel like they'd clash too much with existing names that are still "stream". We might also opportunistically convert a few more names where they feel like they'll substantially improve the coherence of that interim state of things. But we'll put a pause on pushing forward to rename all the "stream" references across the codebase. I just closed #847 and dropped one of the commits from #846, which would have renamed a couple of classes that represent outbound parts of the API (ApiNarrowStream and StreamDestination). For those I want to keep the class names in sync with what we speak to the server, and so pair those renames with starting to use the "channel" names in the API, similar to what we did with ApiNarrowDm. Then the one open PR for this issue at the moment is #848. I'll close that since we're not going to pursue this right now; we'll return to it later. |
This is a follow-up to:
After renaming "stream" to "channel" in the UI, we should do the same throughout our code. The changes for this issue should be purely NFC — user-visible changes are in #630 or other issues.
There are some places in the Zulip server API where the word "stream" appears. The API isn't immediately changing — we'll probably rename all of those eventually, but that'll be a longer migration over time — but even for those, we should still update the names within our code to use the new terminology. For an example, see
UnreadMessagesSnapshot.dms
:Related issues:
Updates on the approach, from discussion on #801:
(1) How to split the changes into commits, and how to group those into PRs: #801 (comment)
and continued at #801 (comment)
(2) A grep command to get a list of types with the word "stream" in their names:
(We also want to rename variables, fields, etc., not just types; but per (1), types are how the rename is organized)
(3) For review on the remaining PRs in this issue, let's use an abbreviated process (#801 (comment)) where we go straight from buddy review to integration review, skipping mentor review and maintainer review. On the one hand there'll generally be less to review in them; and on the other, this sort of cross-codebase refactor is especially prone to developing conflicts, so it's especially good to get them merged soon after they're sent.
The text was updated successfully, but these errors were encountered: