Re: svn commit: r1329015 - in /subversion/trunk/subversion: include/private/svn_ra_private.h libsvn_ra/editor.c libsvn_ra/ra_loader.c libsvn_ra/ra_loader.h

On Mon, Apr 23, 2012 at 4:55 PM, Greg Stein <gstein_at_gmail.com> wrote:
> On Mon, Apr 23, 2012 at 16:21, Hyrum K Wright <hyrum.wright_at_wandisco.com> wrote:
>> On Mon, Apr 23, 2012 at 12:52 PM, Greg Stein <gstein_at_gmail.com> wrote:
>>...
>>> Alright. It sounds like svn_ra.h has two purposes: stuff for libsvn_client,
>>> and stuff for all RA layers to use internally. Ugh.
>>
>> But the capability strings are part of the public API.
>
> Says who? Not server Ev2 support. The RA layer can isolate that, so it
> should not be "part of the public API".
>
> Mergeinfo support? Sure. But Ev2? Nah.

The entire RA interface is part of the public API. The wire protocols
are the public interface to the server, particularly for those who
want to write their own clients and server. These strings are part of
that definition.

>> svn_ra_has_capability() expects them as input, and it makes sense to
>> define them in a public place.  Consumers external to the ra libs can
>> realistically be expected to ask this question, so I don't see a
>> problem with them being in the public space.
>
> I don't see it as realistic. You just use the Ev2 interface and be done with it.

Sure.

My sense of order appreciates the fact that all the capability strings
are #define'd in the same place. As a developer and user, it's nice
to have that stuff together, and the names consistent (you would need
the double-underscore if the macro was private). If you feel strongly
that this should be the one exception to the rule, I won't object
further.

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/