Daniel Berlin wrote:
>>While I don't think that it's necessary for SWIG's sake, should the
>>implementation which delegates to the vtable, and vtable API/functions
>>reflect this variable rename as well (for consistency)?
>
> It doesn't matter. trying to match names up between headers and source
> is a losing war :)
Better for you the writer to do it once than for every reader to have to do it
every time he/she reads the code.
>>How 'bout calling this "mergeinfo" instead of "temp"? As it's added
>>to RESULT, it doesn't really seem temporary...
>
> Again, whatever makes you feel good. If you want to change it, change
> it. It's a temporary variable, used to store the result of a parse.
Of course it's temporary, but every other local variable is temporary too.
Should we call them all "temp"?
>>>+ /* ### todo: I don't understand why the static, file-global
>>>+ variables shared by update and status are called `report_head'
>>>+ and `report_tail', instead of `request_head' and `request_tail'.
>>>+ Maybe Greg can explain? Meanwhile, I'm tentatively using
>>>+ "request_*" for my local vars below. */
>>
>>I think the "report_" prefix is used in ra_dav because it's referring
>>to the payload of a specific type of HTTP request (REPORT). In HTTP
>>client/server apps, "request" usually refers to the entirety of the
>>HTTP request (e.g. including the request method and headers), rather
>>than solely to the request body.
>
> This was copied from log.c :)
That's not much of an excuse as you well know! That comment definitely wants
removing. It's out of date anyway: the variables it refers to don't exist as
far as I can see.
>>>+ /* These get determined from the request document. */
>>>+ svn_revnum_t rev = SVN_INVALID_REVNUM;
>>>+ svn_boolean_t include_parents = 0; /* off by default */
>>
>>This should be svn_boolean_t's constant FALSE rather than 0.
>
> style copied from log.c again :)
> svn_boolean_t discover_changed_paths = 0; /* off by default */
> svn_boolean_t strict_node_history = 0; /* off by default */
Better change it there too, then.
[...]
> As usual, the mod_dav_svn formatting problems are from copying from
> other mod_dav_svn code, so i'm not sure whether we want to be consistent
> or "right".
That's easy: we want both!
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 28 01:26:58 2006