This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
arvind.sirsikar gerrit-no-reply at lists.osmocom.orgPatch Set 15: (8 comments) Hi Neels, Thanks for reviewing the changes. I have addressed all the comments raised by you. Please have a look. Thanks, Aravind Sirsikar https://gerrit.osmocom.org/#/c/655/13/src/rlc.h File src/rlc.h: Line 113: enum egprs_rlcmac_dl_spb { > an enum always contains "values", so this name could be shortened by droppi Done https://gerrit.osmocom.org/#/c/655/13/src/tbf.h File src/tbf.h: Line 424: enum egprs_rlc_dl_reseg_bsn_state egprs_dl_get_data > data is always in a buffer, maybe drop the "_buffer" Done https://gerrit.osmocom.org/#/c/655/13/src/tbf_dl.cpp File src/tbf_dl.cpp: Line 383: bts->bts_data()->dl_arq_type); > I would add another tab indent for the arguments of get_retx_mcs() Done Line 392: m_rlc.block(bsn)->cs_current_trans.to_num(), > I would add another 5-spaces indent for the arguments of LOGP(). If the 80 Done Line 660: need_padding = true; > I find the indenting of the if condition confusing. Done Line 1292: struct gprs_rlc_data *rlc_data = m_rlc.block(bsn); > if you also drop the "_value" from "get_egprs_dl_spb_value", this will fit Done https://gerrit.osmocom.org/#/c/655/13/tests/tbf/TbfTest.cpp File tests/tbf/TbfTest.cpp: Line 1839: if (!(mcs == 6 && demanded_mcs == 3)) > basically the entire function is in an if {} block. Remove the indent by do Done Line 1937: ((mcs == 6) && (demanded_mcs == 3)) || > again, remove indent by if (![...]) return; Done -- To view, visit https://gerrit.osmocom.org/655 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I242afdd8ae7622dec8593b26382ad66bad5b9516 Gerrit-PatchSet: 15 Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Owner: arvind.sirsikar <arvind.sirsikar at radisys.com> Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org> Gerrit-Reviewer: Holger Freyther <holger at freyther.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de> Gerrit-Reviewer: arvind.sirsikar <arvind.sirsikar at radisys.com> Gerrit-HasComments: Yes