On Tue, 05 Dec 2006 23:45:32 +0530, Daniel Rall <firstname.lastname@example.org> wrote:
> On Tue, 05 Dec 2006, Madan S. wrote:
>> On Tue, 05 Dec 2006 00:42:39 +0530, Daniel Rall <email@example.com> wrote:
>> >On Mon, 04 Dec 2006, Madan S. wrote:
> Yes, the name doesn't match exactly what we want to do, as we're not
> actually changing any WC properties here. Too bad the field isn't
> named prop_changes. Hmmmmmm.
> All our options here appear to involving rev'ing the data structure
> and its callers. :-\
> a) Rename wcprop_changes to prop_changes, and use it for both extra
> client -> repos and repos -> WC property edits, with an API contract
> indicating that it must be cleared after the client -> repos prop
> edits. This could be confusing to developers using the structure for
> the first time, but might use less memory.
> b) Add a repos_prop_changes field for client -> repos prop edits.
> Either way, wcprop_changes could REALLY use some better documentation!
I would opt for (a) which would mean one more commit (to rename and change
all occurances of the old name) before we can check this into trunk.
>> But I guess like you said above, setting restting the wcprop_changes
>> a new array (not just setting nelts to 0) would not interfere with the
>> current implementation.
> Really? I didn't notice any place where it might interfere.
> The doc string on apr_array_header_t's nelts field says "The number of
> active elements in the array". It has a separate field, nalloc, which
> represents "The number of elements allocated in the array". My
> understanding of apr_array_header_t is that manipulating nelts is an
> acceptable way to shrink or clear the list (though there's also the
> apr_array_pop() API).
> process_committed_leaf() makes proper use of the nelts field:
> /* Do wcprops in the same log txn as revision, etc. */
> if (wcprop_changes && (wcprop_changes->nelts > 0))
True, but this sanity should not be expected by default. Am not an expert
in APR, so you are most probably right... but I would prefer to err on the
right side :)
>> >A change like this should be committed to trunk first, as it
>> >introduces a general non-WC property setting mechanism which is
>> >decoupled from Merge Tracking.
>> Agree. Will submit a patch to trunk after testing on all ra_xxx's.
>> Does ra_serf need to be tested too?
> Yes, but if you get a patch working with the other three, I can give
> it a whirl over ra_serf.
hmm, I tested it over trunk, and it fails over ra_dav... will check out
the problem and let you know...
Thanks for offering to test over ra_serf.
To unsubscribe, e-mail: firstname.lastname@example.org
For additional commands, e-mail: email@example.com
Received on Thu Dec 7 14:14:09 2006