channel [nfc]: Document methods on ChannelStore; expose debugTopicVisibility directly #931
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This follows the pattern we generally use for other members of ChannelStore and MessageStore, and lets us eliminate the getter returning a ChannelStoreImpl. This pattern -- exposing all the data as members directly on the overall PerAccountStore, delegating to the underlying channel store and message store as needed -- is convenient because it means that code consuming the store doesn't have to worry about which sub-area of the store we've organized a given piece of data into: it's all just methods on the store. Noticed this was here while reviewing a PR that included a new similar deviation from this pattern: zulip#909 (comment)
|
Thanks, LGTM! Merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These members didn't have docs. They have some nontrivial invariants, so can especially benefit from docs.
Then this arrangement of debugTopicVisibility
follows the pattern we generally use for other members of
ChannelStore and MessageStore, and lets us eliminate the getter
returning a ChannelStoreImpl.
This pattern -- exposing all the data as members directly on the
overall PerAccountStore, delegating to the underlying channel store
and message store as needed -- is convenient because it means that
code consuming the store doesn't have to worry about which sub-area
of the store we've organized a given piece of data into: it's all
just methods on the store.
Noticed this was here while reviewing a PR that included a new
similar deviation from this pattern (/cc @PIG208):
#909 (comment)