[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/10102 )

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

Re-enable IWYU checks on HMS module

IWYU previously had some issues with the HMS module[1], probably due to
the Thrift generated classes. Now that IWYU has been upgraded[2] this is
an attempt to re-enable the IWYU checks.  This commit also includes some
associated fixups.

[1]: 
https://github.com/apache/kudu/commit/c09a6296181ecbb7d91687536f70fdbaac9fff5e
[2]: 
https://github.com/apache/kudu/commit/bba339b11795d84da9c9d1473366ca561730c141

Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Reviewed-on: http://gerrit.cloudera.org:8080/10102
Tested-by: Alexey Serbin 
Reviewed-by: Alexey Serbin 
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.h
8 files changed, 43 insertions(+), 37 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved; Verified

--
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: merged
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

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: Code-Review+2


--
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 02:29:23 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

Alexey Serbin has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/10102 )

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


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
--
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: deleteReviewer
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

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.


--
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 01:07:17 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

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.


--
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 00:59:26 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Dan Burkert (Code Review)]

Dan Burkert 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.


--
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 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 00:45:38 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Dan Burkert (Code Review)]

Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/10102

to look at the new patch set (#3).

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

Re-enable IWYU checks on HMS module

IWYU previously had some issues with the HMS module[1], probably due to
the Thrift generated classes. Now that IWYU has been upgraded[2] this is
an attempt to re-enable the IWYU checks.  This commit also includes some
associated fixups.

[1]: 
https://github.com/apache/kudu/commit/c09a6296181ecbb7d91687536f70fdbaac9fff5e
[2]: 
https://github.com/apache/kudu/commit/bba339b11795d84da9c9d1473366ca561730c141

Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.h
8 files changed, 43 insertions(+), 37 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/10102/3
--
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: newpatchset
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Re-enable IWYU checks on HMS module

[Alexey Serbin (Code Review)]

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 2:

Is this still an issue after 72847ab5607b64d64bbe43edd942889e4e9f9fbf was 
committed?


--
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: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 00:24:53 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Dan Burkert (Code Review)]

Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Todd Lipcon,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/10102

to look at the new patch set (#2).

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

Re-enable IWYU checks on HMS module

IWYU previously had some issues with the HMS module[1], probably due to
the Thrift generated classes. Now that IWYU has been upgraded[2] this is
an attempt to re-enable the IWYU checks.  This commit also includes some
associated fixups.

[1]: 
https://github.com/apache/kudu/commit/c09a6296181ecbb7d91687536f70fdbaac9fff5e
[2]: 
https://github.com/apache/kudu/commit/bba339b11795d84da9c9d1473366ca561730c141

Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
---
M build-support/iwyu/iwyu-filter.awk
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
10 files changed, 43 insertions(+), 46 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/10102/2
--
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: newpatchset
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Re-enable IWYU checks on HMS module

[Adar Dembo (Code Review)]

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

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


Patch Set 1: Code-Review+2


--
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: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 18 Apr 2018 20:02:28 +
Gerrit-HasComments: No

[kudu-CR] Re-enable IWYU checks on HMS module

[Dan Burkert (Code Review)]

Hello Adar Dembo, Todd Lipcon,

I'd like you to do a code review. Please visit

http://gerrit.cloudera.org:8080/10102

to review the following change.


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

Re-enable IWYU checks on HMS module

IWYU previously had some issues with the HMS module[1], probably due to
the Thrift generated classes. Now that IWYU has been upgraded[2] this is
an attempt to re-enable the IWYU checks.  This commit also includes some
associated fixups.

[1]: 
https://github.com/apache/kudu/commit/c09a6296181ecbb7d91687536f70fdbaac9fff5e
[2]: 
https://github.com/apache/kudu/commit/bba339b11795d84da9c9d1473366ca561730c141

Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
---
M build-support/iwyu/iwyu-filter.awk
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/hms/hms_client-test.cc
M src/kudu/hms/hms_client.cc
M src/kudu/hms/mini_hms.cc
M src/kudu/hms/mini_hms.h
M src/kudu/integration-tests/master_hms-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
10 files changed, 43 insertions(+), 46 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/10102/1
--
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: newchange
Gerrit-Change-Id: I63e5f1ca6a480f22ede83acde6baea8e2f563e8a
Gerrit-Change-Number: 10102
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Todd Lipcon