Skip to content

actions [nfc]: Namespace actions as statics on a ZulipAction class, and document #1366

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

Merged
merged 2 commits into from
Feb 19, 2025

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Feb 19, 2025

This makes it explicit at each call site that the method being called
is an action, not (for example) an API endpoint binding. The
difference is important in particular because it affects how --
really, whether -- the caller should handle errors.

See discussion from when the similarity in appearance to API endpoint
bindings caused confusion:
https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/.23F1317.20showErrorDialog/near/2080570

Also document the two methods here (other than the private helper) that didn't already have docs.

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Feb 19, 2025
@gnprice gnprice requested a review from chrisbobbe February 19, 2025 04:32
This makes it explicit at each call site that the method being called
is an action, not (for example) an API endpoint binding.  The
difference is important in particular because it affects how --
really, whether -- the caller should handle errors.

See discussion from when the similarity in appearance to API endpoint
bindings caused confusion:
  https://chat.zulip.org/#narrow/channel/516-mobile-dev-help/topic/.23F1317.20showErrorDialog/near/2080570
Like the previous commit, this hopefully helps avoid confusion about
the role of these methods when looking at their call sites.
@chrisbobbe chrisbobbe merged commit a198f72 into zulip:main Feb 19, 2025
1 check passed
@chrisbobbe
Copy link
Collaborator

Thanks, LGTM! Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants