API comments (WAS: Re: Current osmo-pcu patch set)

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/osmocom-net-gprs@lists.osmocom.org/.

Holger Freyther holger at freyther.de
Thu Oct 5 08:47:48 UTC 2017


> On 21. Sep 2017, at 19:14, Max <msuraev at sysmocom.de> 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


More information about the osmocom-net-gprs mailing list