Well, it does seem easier to maintain includes if we rely on the parent header demonstrating intent to provide symbols (e.g. adding a vector to an interface does not require adding includes in all child files).
If it provides significant speedup to build times, it would be very compelling! How do tools like "include what you use" deal with this? On Thu, Jul 30, 2015 at 10:45 AM, Jie Yu <yujie....@gmail.com> wrote: > > > > On July 30, 2015, 4:45 p.m., Vinod Kone wrote: > > > src/tests/containerizer/launcher.hpp, lines 19-37 > > > < > https://reviews.apache.org/r/36929/diff/1/?file=1024924#file1024924line19> > > > > > > why did you remove these headers? > > > > > > i think we decided to explicitly include all the headers that are > used in a file instead of depending on transitive includes. > > > > Jie Yu wrote: > > Is there a discussion somewhere? I think explicitly including all > headers make it hard to maintain (you'll need to adjust the header when the > dependent header changes). Also, it slows down the compilation. > > > > Vinod Kone wrote: > > This is the pattern we followed in the code base. I think it > improves readability and avoids gotchas. For example, it is not clear at > all that the headers you kept provide a string or a vector definition (What > if those headers don't "#include <string>" at a later time?). This is the > relevant blurb from the google style guide > > > > ``` > > You should include all the headers that define the symbols you rely > upon (except in cases of forward declaration). If you rely on symbols from > bar.h, don't count on the fact that you included foo.h which (currently) > includes bar.h: include bar.h yourself, unless foo.h explicitly > demonstrates its intent to provide you the symbols of bar.h. However, any > includes present in the related header do not need to be included again in > the related cc (i.e., foo.cc can rely on foo.h's includes). > > > > ``` > > > > Some relevant discussion: > http://www.mail-archive.com/dev%40mesos.apache.org/msg32694.html > > I have to disagree. C++ is not like Java, we don't have an automated tool > to help you include all necessary headers (java does). As a result, that > makes maintaining the includes very tedious. Any time someone is doing a > refactor, he/she has to manually check all the includes to make sure it's > up-to-date. Unless we have an automated tool to help you include all > necessary headers, I think having less includes makes it easy to maintain. > > The pattern I suggest here is: > ``` > base.hpp > > #include <string> > > class Base { > virtual void foo(const string& str); > }; > > derived.hpp > > #include "base.hpp" > > class Derived : public Base { > virtual void foo(const string& str); > }; > ``` > > We suggest don't include `<string>` in derived.hpp. This does not violate > google style as it states: "unless foo.h explicitly demonstrates its intent > to provide you the symbols of bar.h". I think here, base.hpp clearly > demonstrates its intent to provide you the symbols for all its public > interfaces. > > > - Jie > > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36929/#review93599 > ----------------------------------------------------------- > > > On July 30, 2015, 12:14 a.m., Jie Yu wrote: > > > > ----------------------------------------------------------- > > This is an automatically generated e-mail. To reply, visit: > > https://reviews.apache.org/r/36929/ > > ----------------------------------------------------------- > > > > (Updated July 30, 2015, 12:14 a.m.) > > > > > > Review request for mesos and Vinod Kone. > > > > > > Repository: mesos > > > > > > Description > > ------- > > > > Fixed a few issues in test launcher header. > > > > > > Diffs > > ----- > > > > src/tests/containerizer/launcher.hpp > b80e84196f8b494e6e5a3c96c86f63fe432a7bb0 > > > > Diff: https://reviews.apache.org/r/36929/diff/ > > > > > > Testing > > ------- > > > > make check > > > > > > Thanks, > > > > Jie Yu > > > > > >
