search

eclipse-uprotocol / up-spec

uProtocol Specifications
Apache License 2.0
31 stars 25 forks source link

Use dedicated messages in service operations #163

Closed sophokles73 closed 3 months ago

sophokles73 commented 3 months ago

Changed operation definitions of uProtocol core services to always use dedicated request and response messages as recommended by Protobuf Best Practices.

Removed UStatus from response messages, following recommendations from uProtocol Error Model specification.

This is a follow up for #112

sophokles73 commented 3 months ago

Looks good but please document the newly added fields and messages. Also mention why a field is reserved

I was actually wondering, if we really want/need to reserve the tags for the status fields. FMPOV we would need to do that if we had code that would process messages containing the status field but is then presented with a message that contains a different field under that tag.

Question is: do we have code in the field that expects to find the current message structure as defined on the main branch?

stevenhartley commented 3 months ago

Looks good but please document the newly added fields and messages. Also mention why a field is reserved

I was actually wondering, if we really want/need to reserve the tags for the status fields. FMPOV we would need to do that if we had code that would process messages containing the status field but is then presented with a message that contains a different field under that tag.

Question is: do we have code in the field that expects to find the current message structure as defined on the main branch?

We have v2 code in the field but not v3 (other than in up-android-core but we can fix that) and the v2 code expects the status. Having said that we don't need to keep the placeholder imho.

sophokles73 commented 3 months ago

@stevenhartley I have pushed changes ... Please do not forget to squash when merging.