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