Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10102 )

Change subject: Re-enable IWYU checks on HMS module
......................................................................


Patch Set 3:

> > > Rebased.  I'm a little confused, because I'm not getting iwyu
 > > > errors before _or_ after applying this patch on an internal
 > build
 > > > machine.  We'll see what gerrit says.  I think the code
 > cleanups
 > > in
 > > > this patch are worth landing.
 > >
 > > Yeah, I think the reason you didn't see errors on your local
 > build
 > > machine is that most likely Thrift-related files were already
 > > generated when you ran the 'iwyu' target.  However, in case of
 > > Jenkins slaves for pre-commit builds, the IWYU configuration just
 > > run 'make iwyu' target before as is, without building the rest of
 > > the sources.
 >
 > I just verified on ve0518 with pre-72847ab (i.e. pre-based): if
 > first building everything (i.e. just run 'make') and then run 'make
 > iwyu',  there were no warnings from IWYU.  However, if doing 'make
 > iwyu' on the clean build directory, there were nonsense-style
 > warnings from IWYU.

And yes, with post-72847ab version the problem described above is gone (at 
least at ve0518): if running 'make iwyu' target on the freshly configured build 
directory, IWYU does not report any warnings.


--
To view, visit http://gerrit.cloudera.org:8080/10102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Wed, 25 Apr 2018 01:12:23 +0000
Gerrit-HasComments: No

Reply via email to