Page MenuHomePhabricator

Cleanup doxygen-style comments for function params
Open, LowPublic

Description

deadalnix [11:16 PM]
Honestly, https://reviews.bitcoinabc.org/D1413 is everything that is wrong with things like doxygen

You end up with a ton of redundant infos, that end up being out of date constantly
@param[in]     config     The global config.
There is literaly no information content in the above - except one that is incorrect, namely that the config is "global"
it's a parameter (duh!)
It's a config (ho!)
It's not an out parameter (it's const already)

jasonbcox [11:19 PM]
Heh good point

deadalnix [11:19 PM]
Now there are parameters for which this sin't clear, for instance
> @param[in]     dbp        If non-null, the disk position of the block.

deadalnix [11:20 PM]
except that can be fixed by renaming dbp into `block_position_on_disk` and voila.

jasonbcox [11:20 PM]
For a catch-all config, it's pretty darn self explanatory. I added it for completeness
Dunno if there's anything useful to add to it's description

deadalnix [11:21 PM]
I chose the config because it's especially bad, but in general, if the parameter needs to be explained, it's usually a tell that the API is bad and or naming is fubared
interestingly, doygen actually makes it harder to fix these problems, because now you got to specify new APIs twice
I get that this is something you may want for an API that's considered stable and meant to be used by 3rd party, but besside that, these tools do not seems to me like they achieve something really meaningful

jasonbcox [11:25 PM]
I understand your concern. ultimately I added these because I was confused about fForceProcessing/fRequested. And I have a diff coming up where I add a new param.

It would have been silly to add or fix only one param

deadalnix [11:26 PM]
why ?
I'm not sure what doxygen is able to generate from the function signature
but if it can't generate anything good from there, I'm not sure there is much of a point pursuing this path

tl;dr:

  1. Remove the redundant comments regarding function params (preserving the comments about the function as a whole).
  2. Rename params where appropriate so that they are self-explanatory. In the short term, this may be challenging for params where the design itself is questionable (see fForceProcessing).

Event Timeline

jasonbcox created this task.May 20 2018, 23:52
jasonbcox triaged this task as Low priority.
jasonbcox removed jasonbcox as the assignee of this task.Oct 23 2018, 19:16