Re: svn commit: r18260 - trunk/subversion/libsvn_ra_serf

On 1/27/06, Peter N. Lundblad <peter@famlundblad.se> wrote:
> > + apr_pool_create(&serf_sess->pool, pool);
>
> Should be svn_pool_create, but why do you need a subpool?

See my reply to Greg's post.

(And, I'm trying to avoid using svn-isms for now.)

> I would use _element_ instead of _tag_ in the name, since it adds a whole
> element, not just a tag.

*shrug*

> Also, a docstring here (and on the other functions) would be really
> valuable, because to me it is not obvious exactly what this function does
> and I don't think you should have to read the code to know what a function
> does. One can guess what this function does, but does it, for example,
> escape the value string with XML entities? Note that we have functions to
> create XML tags already in svn_xml.h, but they work on stringbufs, and
> maybe it is more efficient to work on serf buckets directly (or some other
> good reason) in which case this is fine of course.

I am only trying to complete a proof of concept here. Once I can do
the checkout process, I'm going to throw away most of this code and
start again. First, I need to make myself understand what code is
needed and to prove that serf has all of the necessary features.

Once that happens, I'll go through and document everything. Fear not...

> OK, it does not escape the element content. Would it be an improvement to
> do so? Escaping and encoding bugs are quite common...

I'm purposely not escaping or encoding at this point. -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jan 27 18:31:11 2006