> 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.
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 - Vinod ----------------------------------------------------------- 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 > >
