Re: [PATCH] Add necessary copy constructors to libsvn_subr, libsvn_delta (1.3.x, Issue #2172)

Julian, thanks for the thorough review! I can see you took a close
look at my patch. This is much appreciated, thanks.

On 9/28/05, Julian Foad <julianfoad@btopenworld.com> wrote:
> Naming:
> [...]
> I think these new names are OK (ending in "_dup").
Great!

> Which source file:
>
> I think "libsvn_subr/constructors.c" is only appropriate for constructors that
> don't have a better home. That includes svn_log_changed_path_dup(), but I
> think svn_auth_ssl_server_cert_info_dup() should go in auth.c.
Agreed. I'll fix this.

> Some source formatting style nits which we could easily fix up while committing
> the patch:
> [...]
> > Index: subversion/libsvn_subr/constructors.c
> [...]
> > + svn_auth_ssl_server_cert_info_t *new_info =
> > + apr_palloc (pool, sizeof (*new_info));
>
> We tend not to leave an operator (like "=" or "+") on the end of a line; move
> it to the beginning of the next line (indented by two spaces). (Here and in
> two more places.)
Agreed. I'll fix this.

Cheers,

David

--
David James -- http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org