Conversation
|
| // client-side support (e.g. passing RTP packet trailers through to the | ||
| // subscriber instead of stripping them). | ||
| enum Capability { | ||
| CAP_UNKNOWN = 0; |
There was a problem hiding this comment.
what's the intention behind having an Unknown value here?
I don't think we would want/need an unknown value
There was a problem hiding this comment.
I would probably rename to CAP_UNUSED. I think it is useful to mark the default value as unused. In this case, it is not really useful as repeated field will not include the default, but I think it is a good practice to just block off defaults so that it does not get used unintentionally anywhere.
| // subscriber instead of stripping them). | ||
| enum Capability { | ||
| CAP_UNKNOWN = 0; | ||
| CAP_PACKET_TRAILER = 1; |
There was a problem hiding this comment.
should this be more specific, something like CAN_DECODE_PACKET_TRAILER? I think most of these would be to indicate that it can handle something on receive, but I am thinking there may be cases where it indicates encoding/sending capabilities. So, making these explicit would be great.
We can change the name later too as long as the enum value stays the same, but just wanted to bring it up for more clarity/readability.
| // client-side support (e.g. passing RTP packet trailers through to the | ||
| // subscriber instead of stripping them). | ||
| enum Capability { | ||
| CAP_UNKNOWN = 0; |
There was a problem hiding this comment.
I would probably rename to CAP_UNUSED. I think it is useful to mark the default value as unused. In this case, it is not really useful as repeated field will not include the default, but I think it is a good practice to just block off defaults so that it does not get used unintentionally anywhere.
3 participants
No description provided.