[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags This patch adds an option in gflags::GetAllFlags() to show all flags including hidden flags only if the hidden flags were modified from their default values. Testing: ./build.sh gflags 2.2.0-p2 Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Reviewed-on: http://gerrit.cloudera.org:8080/12108 Reviewed-by: Fredy Wijaya Tested-by: Fredy Wijaya --- M buildall.sh A source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch 2 files changed, 151 insertions(+), 2 deletions(-) Approvals: Fredy Wijaya: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 03 Jan 2019 23:08:47 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags This patch adds an option in gflags::GetAllFlags() to show all flags including hidden flags only if the hidden flags were modified from their default values. Testing: ./build.sh gflags 2.2.0-p2 Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 --- M buildall.sh A source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch 2 files changed, 151 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/08/12108/5 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 5: Code-Review+2 Carry Lar's +2. -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 03 Jan 2019 23:06:46 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Dec 2018 15:05:43 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Dec 2018 00:48:54 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags This patch adds an option in gflags::GetAllFlags() to show all flags including hidden flags only if the hidden flags were modified from their default values. Testing: ./build.sh gflags 2.2.0-p2 Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 --- A source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch 1 file changed, 112 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/08/12108/3 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch File source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch: http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@41 PS2, Line 41: + if (i->second->Hidden() && !i->second->Modified()) continue; > I think you can simplify this to I don't think it's the same. When the flag is hidden and show_hidden is true, the condition above becomes false. -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Dec 2018 22:45:27 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch File source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch: http://gerrit.cloudera.org:8080/#/c/12108/2/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@41 PS2, Line 41: + if (i->second->Hidden() && !i->second->Modified()) continue; I think you can simplify this to if (i->second->Hidden() && !show_hidden && !i->second->Modified()) continue; -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Dec 2018 22:30:10 + Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/12108 ) Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags This patch adds an option in gflags::GetAllFlags() to show all flags including hidden flags only if the hidden flags were modified from their default values. Testing: ./build.sh gflags 2.2.0-p2 Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 --- A source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch 1 file changed, 114 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/08/12108/2 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12108 )
Change subject: IMPALA-6521: Patch gflags to have an option to show all flags
including hidden flags
..
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch
File
source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch:
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@63
PS1, Line 63: +TEST(GetAllHiddenFlagsTest, BaseTest) {
> test_bool_hidden is set by this: https://github.com/cloudera/native-toolcha
Thanks
--
To view, visit http://gerrit.cloudera.org:8080/12108
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6
Gerrit-Change-Number: 12108
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 19 Dec 2018 20:55:31 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/12108 )
Change subject: IMPALA-6521: Patch gflags to have an option to show all flags
including hidden flags
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch
File
source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch:
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@63
PS1, Line 63: +TEST(GetAllHiddenFlagsTest, BaseTest) {
> The change itself looks good to me but from this snippet I cannot figure ou
test_bool_hidden is set by this:
https://github.com/cloudera/native-toolchain/blob/master/source/gflags/gflags-2.2.0-patches/0001-Allow-hidden-flags-e.g.-DEFINE_int32_hidden.patch#L274
This is the source tree: https://github.com/fredyw/gflags/tree/show-hidden-flags
--
To view, visit http://gerrit.cloudera.org:8080/12108
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6
Gerrit-Change-Number: 12108
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 19 Dec 2018 02:01:11 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/12108 )
Change subject: IMPALA-6521: Patch gflags to have an option to show all flags
including hidden flags
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch
File
source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch:
http://gerrit.cloudera.org:8080/#/c/12108/1/source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch@63
PS1, Line 63: +TEST(GetAllHiddenFlagsTest, BaseTest) {
The change itself looks good to me but from this snippet I cannot figure out
how the test works, in particular where test_bool_hidden is set. Can you push
the whole tree somewhere so we can look at it?
--
To view, visit http://gerrit.cloudera.org:8080/12108
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6
Gerrit-Change-Number: 12108
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 19 Dec 2018 01:23:36 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12108 Change subject: IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags .. IMPALA-6521: Patch gflags to have an option to show all flags including hidden flags This patch adds an option in gflags::GetAllFlags() to show all flags including hidden flags. Testing: ./build.sh gflags 2.2.0-p2 Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 --- A source/gflags/gflags-2.2.0-patches/0002-Add-show_hidden-parameter-in-GetAllFlags-function.patch 1 file changed, 89 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/08/12108/1 -- To view, visit http://gerrit.cloudera.org:8080/12108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If5b22e4b658759d0fd1a172a85e4e35fa5f5c4b6 Gerrit-Change-Number: 12108 Gerrit-PatchSet: 1 Gerrit-Owner: Fredy Wijaya
