RE: osmo-pcu[master]: Add data structure for ARQ-II in EGPRS DL
Hi Holger,
Line 130: GprsCodingScheme cs_current_trans; I would prefer to cs and cs_current_trans being private here.
The cs_current_trans is made public to align with existing variables like block[RLC_MAX_LEN], len and others to have consistency. will it make sense to have cs and cs_current_trans as private?. We can rename cs to cs_last_tx as suggested in review of other patch. But it will have modification in other source files to address compilation errors. Thanks, Aravind Sirsikar
On 21 Jun 2016, at 09:58, Aravind Sirsikar <Arvind.Sirsikar@radisys.com> wrote:
Hi Holger,
Line 130: GprsCodingScheme cs_current_trans; I would prefer to cs and cs_current_trans being private here.
The cs_current_trans is made public to align with existing variables like block[RLC_MAX_LEN], len and others to have consistency. will it make sense to have cs and cs_current_trans as private?.
We can rename cs to cs_last_tx as suggested in review of other patch. But it will have modification in other source files to address compilation errors.
please keep the communication about the modification in gerrit. If in two years we look at the change we will not know that there was communication out-of-band. If you add this comment to your change I will follow-up in gerrit. thank you for your consideration holger
Hi Holger,
please keep the communication about the modification in gerrit
I have updated comment section on Gerrit for reviews related to ARQ-2 in EGPRS DL. Can you please let me know whether you agree on suggested modifications so that I can re-submit the patches? Thanks, Aravind Sirsikar
participants (2)
-
Aravind Sirsikar -
Holger Freyther