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

Bikeshed: Module names #10

Open
jimmycuadra opened this issue Dec 30, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@jimmycuadra
Copy link
Member

commented Dec 30, 2016

Before publishing this crate, I'd like to settle on good names for the modules for each API endpoint. As always, it's tough to pick good names. Although the naming doesn't affect the functionality of the library, it does affect user experience. We want to strike the right balance between semantics and intuition.

Here are the guidelines I've been using for naming API endpoints' modules:

  • The name should be a verb describing the client is taking, e.g. get_some_resource.
  • Endpoints which are basic CRUD operations should use the prefixes create, get, update, and delete.
  • The prefix set is preferred to create if the resource is a singleton. In other words, when there's no distinction between create and update.

Here are the modules whose names I'm not entirely happy with right now:

  • r0::sync: This contains all endpoints for "getting" events, yet it's named after the one endpoint in that group that doesn't start with get. A contributor previously added a separate events module for the ones prefixed with get, but that left the actual /sync endpoint as the lone endpoint within the top-level sync module. There isn't really a big semantic distinction between "getting" and "syncing" events, so I combined them. events doesn't seem like the right name, cause the idea is that they're endpoints for getting events. After all, everything in Matrix is an event.
  • r0::presence::{get_subscribed_presences, update_presence_subscriptions}: The spec refers to the user's presence subscriptions as a "presence list," but this doesn't seem intuitive to me. However, the current names aren't super obvious either. Maybe we can do better.
  • r0::typing::start_or_stop_typing: The current name feels a bit awkward. Initially it was called set_typing. That name didn't seem quite right, because the user is not setting typing, they're typing. Also, as mentioned above, the set prefix should be used for singleton resources, and while typing is a singleton in concept, using this API creates new m.typing events each time, so it's really more like a "create" operation.

If anyone has thoughts on naming for the above modules, or has concerns or ideas about other modules in the crate, let's use this issue to discuss them.

@vberger

This comment has been minimized.

Copy link
Contributor

commented Dec 30, 2016

Regarding the grouping in modules: a possibility could be to more or less mimic the structure of the spec, so for example end up with a structure like events::sync, events::get::state_for_key, events::send::state_for_key, etc...

This is just a thought, I have no strong conviction about that to be honest.

@jimmycuadra

This comment has been minimized.

Copy link
Member Author

commented Dec 30, 2016

Confusingly, the Swagger docs use yet another categorization of endpoints. I started this crate by going over both the Swagger docs and the spec side by side to come up with categories that I thought were the best combination of "correct" and intuitive.

@vberger

This comment has been minimized.

Copy link
Contributor

commented Dec 31, 2016

Regarding specifically sync::get_state_event_by_event_type and sync::get_state_event_by_state_key (as they are named currently). I find these names misleading.

the associated endpoints are:

  • GET /_matrix/client/r0/rooms/{roomId}/state/{eventType}
  • GET /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}

According to the spec, the first one differs with the second in that it forces the state key to be the empty string, but both will return the state for a given event type and a given state key.

OTOH, the current module names imply in my opinion that one only filters by state key while the other only filters by event type.

@jplatte

This comment has been minimized.

Copy link
Contributor

commented Jan 3, 2017

Re r0::typing::start_or_stop_typing: Does create_typing_event sound good?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.