neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/18351 )
Change subject: Get rid of class GprsCodingScheme ...................................................................... Patch Set 2: Code-Review+1 (3 comments) just a minor cosmetic remark... https://gerrit.osmocom.org/c/osmo-pcu/+/18351/2/src/coding_scheme.h File src/coding_scheme.h: https://gerrit.osmocom.org/c/osmo-pcu/+/18351/2/src/coding_scheme.h@57 PS2, Line 57: bool mcs_is_valid(enum CodingScheme cs); so, here writing 'enum CodingScheme' makes it clear that this is just an enum, but ..... https://gerrit.osmocom.org/c/osmo-pcu/+/18351/2/src/decoding.h File src/decoding.h: https://gerrit.osmocom.org/c/osmo-pcu/+/18351/2/src/decoding.h@43 PS2, Line 43: CodingScheme cs, const uint8_t *data, RlcData *chunks, .... here, you write just CodingScheme, which looks like a complex datatype (class). I think it would be nicer to see 'enum CodingScheme' written out everywhere, because ..... https://gerrit.osmocom.org/c/osmo-pcu/+/18351/2/src/encoding.cpp File src/encoding.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/18351/2/src/encoding.cpp@1114 PS2, Line 1114: switch(mcs_header_type(cs)) { .... here it looks like mcs_header_type() should have been a member function of the CodingScheme class. If it said 'enum' above, that misunderstanding would not come up. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/18351 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie9ce2144ba9e8dbba9704d4e0000a2929e3e41df Gerrit-Change-Number: 18351 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: neels <nhofm...@sysmocom.de> Gerrit-Reviewer: osmith <osm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-Comment-Date: Tue, 19 May 2020 13:54:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
