You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Fuhrmann <st...@wandisco.com> on 2012/09/04 22:03:54 UTC
Consolidating private/svn_subr_private.h
Hi there,
While looking for the appropriate place to declare a few
svn-private constants, I realized that libsvn_subr is the
only lib that comes with multiple private headers.
So, I prepared a patch that moves all their content into
private/svn_subr_private.h with two exceptions:
svn_sqlite.h and svn_magic.h
These are wrappers around 3rd partly libs so I left them
alone.
Right now seems to perfect time for that kind of cleanup
as the old release is reasonably stable and the new
stabilization branch has not been created yet. If there are
no strong objections to the change, I would commit the
patch this weekend.
-- Stefan^2.
--
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*
Re: Consolidating private/svn_subr_private.h
Posted by Greg Stein <gs...@gmail.com>.
Go for it! +1
On Tue, Sep 4, 2012 at 4:03 PM, Stefan Fuhrmann
<st...@wandisco.com> wrote:
> Hi there,
>
> While looking for the appropriate place to declare a few
> svn-private constants, I realized that libsvn_subr is the
> only lib that comes with multiple private headers.
>
> So, I prepared a patch that moves all their content into
> private/svn_subr_private.h with two exceptions:
>
> svn_sqlite.h and svn_magic.h
>
> These are wrappers around 3rd partly libs so I left them
> alone.
>
> Right now seems to perfect time for that kind of cleanup
> as the old release is reasonably stable and the new
> stabilization branch has not been created yet. If there are
> no strong objections to the change, I would commit the
> patch this weekend.
>
> -- Stefan^2.
>
> --
>
> Join us this October at Subversion Live 2012 for two days of best practice
> SVN training, networking, live demos, committer meet and greet, and more!
> Space is limited, so get signed up today!
>
>
RE: Consolidating private/svn_subr_private.h
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Branko Čibej [mailto:brane@wandisco.com]
> Sent: woensdag 5 september 2012 00:57
> To: dev@subversion.apache.org
> Subject: Re: Consolidating private/svn_subr_private.h
>
> On 04.09.2012 23:46, Daniel Shahaf wrote:
> > Stefan Fuhrmann wrote on Tue, Sep 04, 2012 at 22:03:54 +0200:
> >> Hi there,
> >>
> >> While looking for the appropriate place to declare a few
> >> svn-private constants, I realized that libsvn_subr is the
> >> only lib that comes with multiple private headers.
> >>
> > Why is that a problem?
>
> svn_subr contains several quite separate submodules. I think it would be
> a mistake to stuff the declarations into a single private header.
+1
E.g. things like the private fs_path, cache, string and cmdline api are much cleaner in their own targeted header file than in a single shared svn_subr_private.h.
Bert
>
> -- Brane
>
> --
> Certified & Supported Apache Subversion Downloads:
> http://www.wandisco.com/subversion/download
Re: Consolidating private/svn_subr_private.h
Posted by Branko Čibej <br...@wandisco.com>.
On 04.09.2012 23:46, Daniel Shahaf wrote:
> Stefan Fuhrmann wrote on Tue, Sep 04, 2012 at 22:03:54 +0200:
>> Hi there,
>>
>> While looking for the appropriate place to declare a few
>> svn-private constants, I realized that libsvn_subr is the
>> only lib that comes with multiple private headers.
>>
> Why is that a problem?
svn_subr contains several quite separate submodules. I think it would be
a mistake to stuff the declarations into a single private header.
-- Brane
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: Consolidating private/svn_subr_private.h
Posted by Branko Čibej <br...@wandisco.com>.
On 05.09.2012 11:47, Daniel Shahaf wrote:
> Greg Stein wrote on Tue, Sep 04, 2012 at 23:11:24 -0400:
>> On Sep 4, 2012 5:47 PM, "Daniel Shahaf" <d....@daniel.shahaf.name> wrote:
>>> Stefan Fuhrmann wrote on Tue, Sep 04, 2012 at 22:03:54 +0200:
>>>> Hi there,
>>>>
>>>> While looking for the appropriate place to declare a few
>>>> svn-private constants, I realized that libsvn_subr is the
>>>> only lib that comes with multiple private headers.
>>>>
>>> Why is that a problem?
>> Every other library has a single private header. With subr, you gotta go
>> search every time to see if the declaration is in svn_subr_private.h, or
>> some other random header. It's a scattered mess.
>>
> For me it's not a problem as I use ctags.
>
>> Unifying the APIs into a single library header would be a huge improvement.
>>
> Not necessarily, small files are better for some tasks (eg:
> searching/greping) than large ones.
[...]
> How about splitting svn_subr_private.h to svn_spillbuf_private.h,
> svn_checksum_private.h, svn_hash_private.h, and removing
> svn_subr_private.h?
Our public API does not have one header per library, it has one header
per module. I fail to see why the private API shouldn't have the same
model. The argument that having one private header for libsvn_subr makes
it easier to write code spills over to the public APIs, too, and one
might as well argue that we should have a single "svn.h" instead of the
dozen or so public headers that we have. Formally, only our
compatibility guarantees prevent anyone from making such a suggestion,
but I sure hope other considerations would come to mind, too. :)
-- Brane
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: Consolidating private/svn_subr_private.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Tue, Sep 04, 2012 at 23:11:24 -0400:
> On Sep 4, 2012 5:47 PM, "Daniel Shahaf" <d....@daniel.shahaf.name> wrote:
> >
> > Stefan Fuhrmann wrote on Tue, Sep 04, 2012 at 22:03:54 +0200:
> > > Hi there,
> > >
> > > While looking for the appropriate place to declare a few
> > > svn-private constants, I realized that libsvn_subr is the
> > > only lib that comes with multiple private headers.
> > >
> >
> > Why is that a problem?
>
> Every other library has a single private header. With subr, you gotta go
> search every time to see if the declaration is in svn_subr_private.h, or
> some other random header. It's a scattered mess.
>
For me it's not a problem as I use ctags.
> Unifying the APIs into a single library header would be a huge improvement.
>
Not necessarily, small files are better for some tasks (eg:
searching/greping) than large ones.
Given:
% grep ^svn svn_subr_private.h | grep \(
svn_spillbuf__create(apr_size_t blocksize,
svn_spillbuf__get_size(const svn_spillbuf_t *buf);
svn_spillbuf__write(svn_spillbuf_t *buf,
svn_spillbuf__read(const char **data,
svn_spillbuf__process(svn_boolean_t *exhausted,
svn_spillbuf__reader_create(apr_size_t blocksize,
svn_spillbuf__reader_read(apr_size_t *amt,
svn_spillbuf__reader_getc(char *c,
svn_spillbuf__reader_write(svn_spillbuf_reader_t *reader,
svn_stream__from_spillbuf(apr_size_t blocksize,
svn_checksum__from_digest_md5(const unsigned char *digest,
svn_checksum__from_digest_sha1(const unsigned char *digest,
svn_hash__clear(apr_hash_t *hash, apr_pool_t *pool);
svn_hash__get_cstring(apr_hash_t *hash,
svn_hash__get_bool(apr_hash_t *hash,
svn_hash__make(apr_pool_t *pool);
How about splitting svn_subr_private.h to svn_spillbuf_private.h,
svn_checksum_private.h, svn_hash_private.h, and removing
svn_subr_private.h?
> Cheers,
> -g
Re: Consolidating private/svn_subr_private.h
Posted by Greg Stein <gs...@gmail.com>.
On Sep 4, 2012 5:47 PM, "Daniel Shahaf" <d....@daniel.shahaf.name> wrote:
>
> Stefan Fuhrmann wrote on Tue, Sep 04, 2012 at 22:03:54 +0200:
> > Hi there,
> >
> > While looking for the appropriate place to declare a few
> > svn-private constants, I realized that libsvn_subr is the
> > only lib that comes with multiple private headers.
> >
>
> Why is that a problem?
Every other library has a single private header. With subr, you gotta go
search every time to see if the declaration is in svn_subr_private.h, or
some other random header. It's a scattered mess.
Unifying the APIs into a single library header would be a huge improvement.
Cheers,
-g
Re: Consolidating private/svn_subr_private.h
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Tue, Sep 04, 2012 at 22:03:54 +0200:
> Hi there,
>
> While looking for the appropriate place to declare a few
> svn-private constants, I realized that libsvn_subr is the
> only lib that comes with multiple private headers.
>
Why is that a problem?
Re: Consolidating private/svn_subr_private.h
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Tue, Sep 4, 2012 at 10:03 PM, Stefan Fuhrmann <
stefan.fuhrmann@wandisco.com> wrote:
> Hi there,
>
> While looking for the appropriate place to declare a few
> svn-private constants, I realized that libsvn_subr is the
> only lib that comes with multiple private headers.
>
> So, I prepared a patch that moves all their content into
> private/svn_subr_private.h with two exceptions:
>
> svn_sqlite.h and svn_magic.h
>
> These are wrappers around 3rd partly libs so I left them
> alone.
>
> Right now seems to perfect time for that kind of cleanup
> as the old release is reasonably stable and the new
> stabilization branch has not been created yet. If there are
> no strong objections to the change, I would commit the
> patch this weekend.
>
> Since the majority of people feel that this would not be
a beneficial move, I will leave the header structure as
it is.
Thanks to everyone making his voice heard!
-- Stefan^2.
--
*
Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*