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

Change subject: [thrift] Suppress Thrift warnings using -nowarn option
......................................................................


Patch Set 4: Code-Review-1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/23063/4/cmake_modules/FindThrift.cmake
File cmake_modules/FindThrift.cmake:

http://gerrit.cloudera.org:8080/#/c/23063/4/cmake_modules/FindThrift.cmake@127
PS4, Line 127:         -nowarn
I don't think doing this in a blanket-style manner like you did here is a good 
idea.

With this update, all warnings in all invocations of the compiler (even if 
anybody adds unrelated Thrift stuff in a completely different place) would be 
silenced, not only in the particular rogue Hive file you are trying to target.

Even the Thrift compiler's help message tells you that's not a good idea:

  -nowarn     Suppress all compiler warnings (BAD!)


If adding the `-nowarn` option, it's better doing so for a particular instance 
of the THRIFT_GENERATE_CPP command, but not hard-code it into all 
THRIFT_GENERATE_CPP targets, no.

To address this properly, please consider introducing extra options for 
THRIFT_GENERATE_CPP custom command and adding `-nowarn` option for 
THRIFT_GENERATE_CPP() for the hive_metastore.thrift file.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdb8b650f9b8ed1cdea9dd0cf1a80ef264ccc7d7
Gerrit-Change-Number: 23063
Gerrit-PatchSet: 4
Gerrit-Owner: KeDeng <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: KeDeng <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Comment-Date: Wed, 25 Jun 2025 15:53:38 +0000
Gerrit-HasComments: Yes

Reply via email to