-
Notifications
You must be signed in to change notification settings - Fork 17
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
Improving event type values in the Notification API #197
Comments
What makes most sense to me is:
Some examples from the schemas we use in WebThings are "alarm", "doublePressed", "longPressed", "overheated". However, looking at other event names in web specifications it appears that using past-tense verb phrases is not a common approach to naming events. A more common approach would seem to be to simply call them "create", "update" and "delete" (Convention B, current) as with "click", "install", "copy", "open", "close" etc. Or possibly "thingcreate", "thingupdate", "thingdelete" like "compositionupdate", "timeupdate", "contentdelete". The latter sounds a bit clunky though. Note that I used camelCase for the interaction affordance names in the Thing Description (as this is the convention in JavaScript/JSON), but event names themselves (e.g. to use over the wire or as the value of the type variable in the URL) tend to be all lower case with no separators between words. |
Current spec has nouns for events: thingDeletion, thingUpdate, thingCreate. Is this an acceptable convention now, and can we close this issue? |
This issue is about the event type values used by the API. We currently have |
I still think |
I am in favour of this and suggest we continue using verbs. I'm just concerned with extensibility i.e. if an implementation wants to use the same API for other event types. With that in mind, |
FWIW |
True. Any particular reason on not having a separator between the words? snake_case or kebab-case are both case-insensitive and suitable. Having no separator at all makes it hard to read or even programatically separate the object (thing) from the state (created/updated/deleted). |
@farshidtz wrote:
Mostly just because that's what other web specifications do for event names. It's also what we do for operation names in the Thing Description specification. I don't have a strong opinion about it, just following convention. |
I created a PR to change the naming convention to Convention C. Please review. |
The Notification API supports three types of events.
The event types has specific names used in URI variables. These were changed recently and a pending proposal suggests yet another change. But before doing so, it is better to discuss pros and cons of every naming convention.
Convention A
WD1 used the following event types:
created_td
,updated_td
,deleted_td
Note: This is before changing
td
tothing
/things
across APIs. In alignment to the current spec, those types would be:created_things
,updated_things
,deleted_things
Convention B (current)
The types were recently changed in response to feedback in F2F asking for simplifications (#155). The new types are similar to
add
andremove
from the SSE spec examples:create
,update
,delete
Concerns:
thing
prefix/suffix as in conventions A and C resolves that. On the other hand, we could assume that default events always contain TDs and add prefixes/suffixes for new possible content types.Convention C
One new proposal (#159 superseded by #184) suggests:
thing_created
,thing_updated
,thing_deleted
This is an improvement to B but past tense verbs as prefixes is not so common and less readable (subscribing to thing created events?).
Taking a look at MQTT topic best practices and the event examples in TD spec, it looks like nouns are a more common choice. E.g. temperature, stats, status
That takes us to conventions D and E:
Convention D
thing_creation
,thing_update
,thing_deletion
This is very readable: subscribe to thing creation events:
/events?type=thing_creation
(SSE):Naively mapping it to an MQTT topic:
things/creation/{id}
Convention E
Same as D but without the prefix:
creation
,update
,deletion
For future extensions, I can think of:
The text was updated successfully, but these errors were encountered: