On Mon, 19 Mar 2007, Erik Huelsmann wrote:
> On 3/19/07, Ivan Zhakov <firstname.lastname@example.org> wrote:
> >On 3/17/07, Erik Huelsmann <email@example.com> wrote:
> >> In libsvn_wc, we have a number of patterns which are extremely common,
> >> yet verbosely repeated every time. One of them is addressed in the
> >> patch attached.
> >> The pattern is:
> >> - get an entry using svn_wc_entry()
> >> - check for a non-NULL entry and,
> >> return a not-versioned error if the entry *is* NULL
> >> The only thing I could figure out why this would be is that we want to
> >> have file and line numbers in the error message, so, I introduced a
> >> macro and a new function.
> >> 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.
> >> Comments?
> >I like this idea, but I prefer to add this helper locally to libsvn_wc
> >and libsvn_client. Because exporting and maintaining such helpers
> >isn't good way. Interface of library should be minimal as possible. So
> >I propose to create svn_wc__ventry_ex() and
> >Yes this introduce *some* code duplication, but I think it's better
> >than exporting helpers from our library.
> But we already do in the context of error handling: see svn_error__locate().
> I also wouldn't like to export helpers, but I think it's 'madness' to
> start maintaining code in several layers separately. What if we were
> to change error creation to move away from global variables and use
> the same scheme as the one here?
> Next to that, I tink that documenting something as 'internal' should
> be enough for people not to use it... It's not like they have no other
> You are aware that all svn_*__ symbols are exported from the libraries
> (at least on Linux) too?
As Vlad suggested on IRC, we now have the include/private/ area. Paul
and I recently created a svn_wc_private.h in there for a
project-private API that we don't want exported by our public header
Our svn_*__* APIs can start following this pattern. HACKING should be
updated to recommend its usage.
Received on Mon Mar 19 22:19:52 2007
- application/pgp-signature attachment: stored