Patch Set 1: Code-Review+1 (4 comments)
Thnaks a lot for your contribution! It is very much appreciated, please forgitve me missing it in the patch review so far. It's not usual that it waits for three weeks. If you hsould see that again, feel free to send a "ping" either by private mail or here as a "reply" on the patch. https://gerrit.osmocom.org/#/c/2817/1/src/lower_mac/tetra_lower_mac.c File src/lower_mac/tetra_lower_mac.c: Line 106: [DPSAP_SCH_S] = { //DMO Synchronization Burst Block 1 I would appreciate if you could avoid introducing // style comments, we don't use them in osmo-tetra (or the GSM related projects). It's not a critical requirement, but I would appreciate it. https://gerrit.osmocom.org/#/c/2817/1/src/phy/tetra_burst.c File src/phy/tetra_burst.c: Line 305: #if 0 /* not used */ not used by whom? why? I think more context is needed here. also, it seems like an unrelated change to DMO support, right? If so, please have separate patch explaining rationale in commitlog. Line 367: break; extra whitspace at end of break https://gerrit.osmocom.org/#/c/2817/1/src/phy/tetra_burst_sync.c File src/phy/tetra_burst_sync.c: Line 147: case (TETRA_TRAIN_NORM_1): /* fall through */ why do we add patenthesis here? The other cases work without? -- To view, visit https://gerrit.osmocom.org/2817 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifa5521d7313595384e74dd790a56550755b93fe9 Gerrit-PatchSet: 1 Gerrit-Project: osmo-tetra Gerrit-Branch: laforge/sq5bpf-rebase-20161218 Gerrit-Owner: allesklar2 <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-HasComments: Yes
