[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] svn_wc_ventry(_ex) to eliminate heavily repeated code

From: Daniel Rall <dlr_at_collab.net>
Date: 2007-03-19 18:41:54 CET

On Sat, 17 Mar 2007, Malcolm Rowe wrote:

> On Sat, Mar 17, 2007 at 12:14:21AM +0100, Erik Huelsmann wrote:
> > The function svn_wc_ventry_ex (ventry as for 'valid entry') which
> > takes a file name and a line number. The macro svn_wc_ventry()
> > forwards to the function, adding the filename and lineno.
> >
> > I know it isn't our usual pattern, but it's one of the ways I see to
> > make libsvn_wc a bit more readable.
> >
> > BTW: This patch has 2 failing tests, because of changed error output.
> > No real problems, as far as I can tell.

I like it!

> I like this approach, although I wonder whether it's appropriate to
> consolidate the existing error messages into one "%s is not under version
> control" messages?
>
> I'd also prefer a more explicit function name: when I saw it first, I
> guessed 'ventry' meant 'valid' or 'virtual' (or 'ventricle'). Perhaps
> 'svn_wc_entry_versioned()', making it clearly a variant of svn_wc_entry()?

I was also perplexed by the "v" prefix. I prefer Malcolm's suggested
svn_wc_entry_versioned() name.

svn_wc_ventry_ex() is documented as "private", so should use our __
notation. In this case, I'd also prefer to stick with the _internal
suffix used by some of our other APIs than introduce the _ex
convention.

  • application/pgp-signature attachment: stored
Received on Mon Mar 19 18:46:41 2007

This is an archived mail posted to the Subversion Dev mailing list.