Attention is currently required from: osmith, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-cbc/+/28602 )
Change subject: Add initial SBc-AP support to osmo-cbc ......................................................................
Patch Set 11:
(3 comments)
File include/osmocom/cbc/cbc_data.h:
https://gerrit.osmocom.org/c/osmo-cbc/+/28602/comment/306ccc6a_175b3cd4 PS10, Line 23: CBC_PEER_PROTO_SBcAP
It's really arguable whether it makes sense to add a comma at the end of the last one.
Well, there was one before this patch. And it's generally a good practice making patches adding new entries easier to read. In any case, I am not going to block because of that.
File include/osmocom/cbc/internal.h:
https://gerrit.osmocom.org/c/osmo-cbc/+/28602/comment/160d2fad_ea0249b6 PS10, Line 30: enum sbcap_server_event {
No, because it's not (or will not) be exactly the same set, nor exactly the same meaning.
If the meaning is not the same, why names and comments are? Just wondering.
File src/sbcap_server_fsm.c:
https://gerrit.osmocom.org/c/osmo-cbc/+/28602/comment/9df0ccdf_c4ee5b6e PS8, Line 303: //else
commented out code, remove?
Ack. Neither it's marked as FIXME nor as TODO. It's better to use #if 0 ... #else ... #endif here if it should be kept.
-
fixeria