On Thu, 16 Feb 2006, Justin Erenkrantz wrote:
> On 2/16/06, Daniel Rall <dlr@collab.net> wrote:
> > Comment doesn't add a lot of additional value. I recommend either
> > removing it, or being a bit more verbose:
>
> I'll remove the comment.
>
+1. I think our requirement in HACKING to "document everything" is too
strict, btw.
> > Need a #define for the magic number?
>
> I'm still trying to determine what the 'right' number is. 2KB is
> certainly not large enough as that tends to just make us repeat reads
> (i.e. we typically have more than 2KB available when we go to read).
>
> So, I expect the magic number to change as I quantify this loop's
> performance more...
>
Is that an argument for not having a #define?
> > > + /* We need to move the prop_ns, prop_name, and prop_val into the
> > > + * same lifetime as the dir->pool.
> > > + */
> > > + ns_t *ns, *ns_name_match;
> > > + int found = 0;
> >
> > Why not use a svn_boolean_t here to represent the flag, rather than an
> > int?
>
> Again, some of this XML code may end up in serf which won't have a boolean flag.
>
And in that case, changing it to int will be required to make it compile.
OTOH, there is nothing that will guarantee that we clean this up later if
it stays in the svn codebase. Don't get me wrong. I'm not arguing about
this for the fun of it. But looking in the libsvn_ra_dav code (for
example) shows lots of places with "### no error handling for now" or
similar that never got cleaned up...
Regards,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Feb 17 08:53:22 2006