> On 21. Sep 2017, at 19:14, Max <[email protected]> wrote:
Hi Max, > >> "API" documentation: >> >> Comments can be helpful and it is good to explain on a high level what a >> function will do and what the side-effects will be. But in an application >> I think writing: >> >> \param[in,out] bts Pointer to BTS struct >> >> adds not enough value. First it is wrong as the type is not "BTS" but it >> is the "C" gprs_rlcmac_bts. > > It would be more helpful if this sort of feedback would be provided via gerrit > because there it's easy to point to particular line of code. That way we can > be sure > that such oversights are quickly corrected. Thank you. https://gerrit.osmocom.org/#/c/3905/8/src/gprs_rlcmac_ts_alloc.cpp https://gerrit.osmocom.org/#/c/3906/10/src/gprs_rlcmac_ts_alloc.cpp https://gerrit.osmocom.org/#/c/3912/5/src/gprs_rlcmac_ts_alloc.cpp https://gerrit.osmocom.org/#/c/3913/6/src/gprs_rlcmac_ts_alloc.cpp https://gerrit.osmocom.org/#/c/3930/5/src/gprs_rlcmac_ts_alloc.cpp https://gerrit.osmocom.org/#/c/3934/4/src/gprs_rlcmac_ts_alloc.cpp https://gerrit.osmocom.org/#/c/3807/10/src/gprs_rlcmac_ts_alloc.cpp https://gerrit.osmocom.org/#/c/3935/4/src/gprs_rlcmac_ts_alloc.cpp the above list might not be complete but all of them show parameter comments that do not carry a lot of value. I think it is a net loss as: * It took you time to write it * It makes a wrong statement, especially as gprs_rlcmac_bts and BTS are different types. * It takes time to review, it takes time to go through the comments to find them. Given the amount of these comments I think it is best to address them with an out-of gerrit discussion. holger
