Re: Review of lock-many API
From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 8 Apr 2014 12:21:50 +0100 (BST)
Thanks for the other fixes and doc tweaks you committed related to this.
Philip Martin wrote:
> Julian Foad writes:
Oops, sorry about that. (I copied it from a terminal screen, forgetting that the diff headers should contain tabs.)
>> +/** ### Document here not what the callers will do but what this function
What I mean is, as a matter of documentation style, to document this more in terms of itself and depend less on its current callers and their environments.
>> - * @a path and @a lock are allocated in the result_pool passed to
For example: The reader may not be thinking immediately about the two callers named here, having come here from one of the (at the time of writing) six other direct callers. Two of those don't have a result_pool as such and don't clear the error as such. What is written is still true, it just could be more independent. Perhaps like this:
* @a lock contains the new (as opposed to the previous) lock on @a path
Describing the error that way additionally gives me the idea that it should be passed by a 'const' pointer, as this function is not supposed to modify it.
And it's not necessarily a FS-layer error: it could be a repos-layer error from the pre-lock or pre-unlock hook. So maybe rename it to just 'err'?
> That documents the what the callback gets given, the callback can do
This is a promise about what those particular callers
Of course sometimes it's expediant to document at a place like this the common behaviours that should be
This is an archived mail posted to the Subversion Dev mailing list.