> -----Original Message-----
> From: Greg Stein [mailto:gstein_at_gmail.com]
> Sent: dinsdag 2 maart 2010 0:18
> To: Julian Foad
> Cc: dev_at_subversion.apache.org
> Subject: Re: [PATCH] WC-NG properties API - complete doc strings
>
> On Mon, Mar 1, 2010 at 07:23, Julian Foad <julian.foad_at_wandisco.com>
> wrote:
> > For review, please. The patch below aims to provide complete doc
> > strings for (the existing implementation of) the WC-NG properties API.
>
> Looks great, thanks.
>
> > I assume "properties" refers to regular, versioned properties
> > throughout.
>
> Yes.
>
> > I checked the implementation with regard to returning an error when the
> > node is not found.
>
> It should always return an error. We were very inconsistent before,
> and my intent was to error in all cases when a node is not found
> (thus, my slight concern around the read_kind() function).
>
> If it doesn't error, then that should be considered a bug.
>
> > I believe svn_wc__db_base_get_props() is meant to return an empty
> > (non-NULL) hash to represent "no properties", both because that's
>
> Correct.
>
> > consistent with the input and outputs of the rest of these functions and
> > because the rest of these functions never write "null" to the
> > "properties" column of the BASE table when writing an empty set of
> > properties. The non-nullness of that SQL column is not (yet) documented
> > so I am not certain that there are no other functions that could write
> > "null".
>
> A null value in ACTUAL_NODE.properties means "no changes w.r.t the
> pristine properties". Any value in there is the complete set of
> (changed/deleted/added) properties.
I would remove that part of '(changed/deleted/added)' as it also contains
the unmodified properties. It is really the complete set.
>
> I don't think that a null value in WORKING_NODE or BASE_NODE makes
> sense. We could interpret that as "incomplete", or as a short-form for
> "no properties". iirc, a skel for empty-property-hash is "()" (ie.
> just 2 bytes).
I think the current code assumes that the properties in WORKING_NODE are
NULL, for the base-deleted case. (And not sure about deleted from a copy,
but if that case is NULL it would be a bug... The tests completed by the end
of October, so they should be ok)
Bert
Received on 2010-03-02 12:21:36 CET