From: David Aguilar <davvid@gmail.com>
Date: Sun, 07 Sep 2014 20:30:51 GMT
Subject: Re: [RFC PATCH v2 2/2] headers: include dependent headers
Message-ID: <20140907203049.GA61326@gmail.com>
In-Reply-To: <20140907194917.GB1611@google.com>

On Sun, Sep 07, 2014 at 12:49:18PM -0700, Jonathan Nieder wrote:
> David Aguilar wrote:
> 
> > Add dependent headers so that including a header does not
> > require including additional headers.
> 
> I agree with this goal, modulo the compat-util.h caveat.  Thanks
> for working on it.
> 
> [...]
> > --- a/archive.h
> > +++ b/archive.h
> > @@ -1,6 +1,7 @@
> >  #ifndef ARCHIVE_H
> >  #define ARCHIVE_H
> >  
> > +#include "cache.h"
> >  #include "pathspec.h"
> >  
> >  struct archiver_args {
> 
> I'm less happy about the way of achieving that goal.  Here's an
> alternative.  Advantages:
> 
>  * (fully expanded) headers stay small
> 
>  * fewer other headers included as a side-effect of including one
>    header, so callers are more likely to remember to #include the
>    headers defining things they need (which makes later refactoring
>    easier)
> 
>  * circular header dependencies are harder to produce
> 
> If this seems like a good direction to go in, I can finish the patch
> later today

Yes, please, that would be sweet.

Would you mind squashing in your sug to patch 1/2 as well when resending?
It seems like a nice improvement all around.

RE: pre-compiled headers -- that might be a nice follow-up to
this series. I'm not very familiar with Windows so I don't know
if it would be doable on mingw, cygwin, and msvc et al. but if
it helps other platforms then it could be a nice feature.

Thanks Jonathan,
-- 
David