Re: [PATCH] Issue #1295

On 8 May 2003 cmpilato@collab.net wrote:

> > + /* I couldn't find an apr_array_delete -- MYG. */
> > + if (ci->nelts > 1)
> > + ci->elts[i] = ci->elts[ci->nelts - 1];
> > + ci->nelts--;
> > + }
> > + }
> > +
>
> I think I'd feel better about seeing this as a memmove().

The memmove would be slower but it would keep the order. That's why I did
this before the sort, so I could use the more efficient trick.

If you want, I'll happily change it to a memmove(). Whatever you guys
want...

> > +static const char *
> > +get_item_path (svn_client_commit_item_t *item,
> > + struct log_msg_baton *lmb,
> > + apr_pool_t *pool)
>
> Icky poo. Please use the same style as the rest of the function
> headers on a per-file basis. For this file (as for all of Subversion),
> we have a single space before our parameter names, like:

I thought I was. Sorry.

Want me to change it and re-submit patch?

> static const char *
> get_item_path (svn_client_commit_item_t *item,
> struct log_msg_baton *lmb,
> apr_pool_t *pool)
>
> (Unfortunately, I don't have the cycles to review the rest of the
> patch, but the use of the commit_item->flags was quite clever -- I
> very much like that mechanism.)

Thanks. It comes from years of messing with 8051 code and using every last
bit cleverly. ;-)

This is my first "real" patch submission. Sorry if I screwed it up...
Anyhow, if you want I'll re-submit fixing the formatting (it's kind of
hard to change styles when I've been using my style for the last 8-9 years
or so).

As for the, memmove(). It's academic. Anyone have a suggestion one way or
another?

L8r,
Mark G.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 9 01:56:47 2003