You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/11/09 13:51:28 UTC
Re: svn commit: r40417 - in trunk: . subversion/include
subversion/include/private subversion/libsvn_client subversion/libsvn_subr
subversion/libsvn_wc
On Sun, Nov 8, 2009 at 05:17, Stefan Sperling <st...@elego.de> wrote:
> On Sun, Nov 08, 2009 at 07:30:36AM +0100, Branko Cibej wrote:
>> Verdict: -1, please revert this change.
>
> +1 on revert, and simply removing #include <svn_debug.h> from
> svn_types.h and requiring people who use SVN_DBG to include svn_debug.h
> manually.
I put the include into svn_types.h to make our lives easier. Without
that, then we'll end up with svn_debug.h scattered around (as noted by
Arfrever's removal of them).
There should be NO PROBLEM including a private header from a public
one, if that inclusion only happens *for us*.
And can I just say: -1 also on all these changes. This is a pretty
crappy "solution" to whatever the original problem might have been.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415784
Re: svn commit: r40417 - in trunk: . subversion/include
subversion/include/private subversion/libsvn_client subversion/libsvn_subr
subversion/libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Nov 13, 2009 at 00:06, Arfrever Frehtes Taifersar Arahesis
<ar...@gmail.com> wrote:
> 2009-11-09 14:51:28 Greg Stein napisał(a):
>> On Sun, Nov 8, 2009 at 05:17, Stefan Sperling <st...@elego.de> wrote:
>> > On Sun, Nov 08, 2009 at 07:30:36AM +0100, Branko Cibej wrote:
>> >> Verdict: -1, please revert this change.
>> >
>> > +1 on revert, and simply removing #include <svn_debug.h> from
>> > svn_types.h and requiring people who use SVN_DBG to include svn_debug.h
>> > manually.
>>
>> I put the include into svn_types.h to make our lives easier. Without
>> that, then we'll end up with svn_debug.h scattered around (as noted by
>> Arfrever's removal of them).
>
> Why don't you use svn_private_config.h? It's included in even more files
> than svn_types.h:
>
> $ grep -Er '#include[[:space:]]+"svn_private_config\.h"' subversion | grep -v /\.svn/ | wc -l
> 273
> $ grep -Er '#include[[:space:]]+"svn_types\.h"' subversion | grep -v /\.svn/ | wc -l
> 168
Hadn't thought about that solution. That should work fine.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2417414
Re: svn commit: r40417 - in trunk: . subversion/include
subversion/include/private subversion/libsvn_client subversion/libsvn_subr
subversion/libsvn_wc
Posted by Bert Huijben <rh...@sharpsvn.net>.
On Fri, Nov 13, 2009 at 6:06 AM, Arfrever Frehtes Taifersar Arahesis
<ar...@gmail.com> wrote:
> 2009-11-09 14:51:28 Greg Stein napisał(a):
>> On Sun, Nov 8, 2009 at 05:17, Stefan Sperling <st...@elego.de> wrote:
>> > On Sun, Nov 08, 2009 at 07:30:36AM +0100, Branko Cibej wrote:
>> >> Verdict: -1, please revert this change.
>> >
>> > +1 on revert, and simply removing #include <svn_debug.h> from
>> > svn_types.h and requiring people who use SVN_DBG to include svn_debug.h
>> > manually.
>>
>> I put the include into svn_types.h to make our lives easier. Without
>> that, then we'll end up with svn_debug.h scattered around (as noted by
>> Arfrever's removal of them).
>
> Why don't you use svn_private_config.h? It's included in even more files
> than svn_types.h:
>
> $ grep -Er '#include[[:space:]]+"svn_private_config\.h"' subversion | grep -v /\.svn/ | wc -l
> 273
> $ grep -Er '#include[[:space:]]+"svn_types\.h"' subversion | grep -v /\.svn/ | wc -l
> 168
I like the idea of using svn_private_config.h, but these numbers by
itself don't tell the full story. Almost all of our headers include
svn_types.h, so you don't have to include it directly. And you are
only counting direct includes.
Bert
>
> --
> Arfrever Frehtes Taifersar Arahesis
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2418141
Re: svn commit: r40417 - in trunk: . subversion/include
subversion/include/private subversion/libsvn_client subversion/libsvn_subr
subversion/libsvn_wc
Posted by Arfrever Frehtes Taifersar Arahesis <Ar...@GMail.Com>.
2009-11-09 14:51:28 Greg Stein napisał(a):
> On Sun, Nov 8, 2009 at 05:17, Stefan Sperling <st...@elego.de> wrote:
> > On Sun, Nov 08, 2009 at 07:30:36AM +0100, Branko Cibej wrote:
> >> Verdict: -1, please revert this change.
> >
> > +1 on revert, and simply removing #include <svn_debug.h> from
> > svn_types.h and requiring people who use SVN_DBG to include svn_debug.h
> > manually.
>
> I put the include into svn_types.h to make our lives easier. Without
> that, then we'll end up with svn_debug.h scattered around (as noted by
> Arfrever's removal of them).
Why don't you use svn_private_config.h? It's included in even more files
than svn_types.h:
$ grep -Er '#include[[:space:]]+"svn_private_config\.h"' subversion | grep -v /\.svn/ | wc -l
273
$ grep -Er '#include[[:space:]]+"svn_types\.h"' subversion | grep -v /\.svn/ | wc -l
168
--
Arfrever Frehtes Taifersar Arahesis
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2417412
Re: svn commit: r40417 - in trunk: . subversion/include
subversion/include/private subversion/libsvn_client subversion/libsvn_subr
subversion/libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
On Mon, Nov 9, 2009 at 09:01, Branko Čibej <br...@xbc.nu> wrote:
> Greg Stein wrote:
>> On Sun, Nov 8, 2009 at 05:17, Stefan Sperling <st...@elego.de> wrote:
>>
>>> On Sun, Nov 08, 2009 at 07:30:36AM +0100, Branko Cibej wrote:
>>>
>>>> Verdict: -1, please revert this change.
>>>>
>>> +1 on revert, and simply removing #include <svn_debug.h> from
>>> svn_types.h and requiring people who use SVN_DBG to include svn_debug.h
>>> manually.
>>>
>>
>> I put the include into svn_types.h to make our lives easier. Without
>> that, then we'll end up with svn_debug.h scattered around (as noted by
>> Arfrever's removal of them).
>>
>> There should be NO PROBLEM including a private header from a public
>> one, if that inclusion only happens *for us*.
>>
>
> Well ... Yeah, I guess, sure, as long as no-one who builds tools on top
> of an installed Subversion #defines SVN_DEBUG in their build. Now I know
> of no good reason to do such a thing; but it could happen.
>
> So in the end, its a choice between telling said user in an ugly way
> that she should leave off defining SVN_DEBUG (file not found:
> private/svn_debug.h), or just silently ignoring that. Or generating
> svn_types.h.
>
> Frankly at this point I can't be bothered to think things through enough
> to care; as long as the "fix" got reverted.
hehe...
Well, nobody should be defining symbols in our namespace. I think our
API story has always been pretty clear on that. So I'm quite fine with
it failing for people when they do that.
And meanwhile: we get easier development.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415793
Re: svn commit: r40417 - in trunk: . subversion/include
subversion/include/private subversion/libsvn_client subversion/libsvn_subr
subversion/libsvn_wc
Posted by Branko Cibej <br...@xbc.nu>.
Greg Stein wrote:
> On Sun, Nov 8, 2009 at 05:17, Stefan Sperling <st...@elego.de> wrote:
>
>> On Sun, Nov 08, 2009 at 07:30:36AM +0100, Branko Cibej wrote:
>>
>>> Verdict: -1, please revert this change.
>>>
>> +1 on revert, and simply removing #include <svn_debug.h> from
>> svn_types.h and requiring people who use SVN_DBG to include svn_debug.h
>> manually.
>>
>
> I put the include into svn_types.h to make our lives easier. Without
> that, then we'll end up with svn_debug.h scattered around (as noted by
> Arfrever's removal of them).
>
> There should be NO PROBLEM including a private header from a public
> one, if that inclusion only happens *for us*.
>
Well ... Yeah, I guess, sure, as long as no-one who builds tools on top
of an installed Subversion #defines SVN_DEBUG in their build. Now I know
of no good reason to do such a thing; but it could happen.
So in the end, its a choice between telling said user in an ugly way
that she should leave off defining SVN_DEBUG (file not found:
private/svn_debug.h), or just silently ignoring that. Or generating
svn_types.h.
Frankly at this point I can't be bothered to think things through enough
to care; as long as the "fix" got reverted.
-- Brane
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2415789