You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2017/04/07 17:11:36 UTC
[kudu-CR] Add --redact=none flag option for specifying no redaction
Hello Hao Hao, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/6590
to review the following change.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Add --redact=none flag option for specifying no redaction
It's still possible to specify no redaction via --redact='', but I was
only able to determine how to do that by reading the code. I think this
makes it a bit easier to use.
Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
---
M src/kudu/util/flags.cc
1 file changed, 18 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/6590/1
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Patch Set 4:
The differences is that the flags are now getting reset back to the default value every time the validator runs. During tests, the validator would get run twice: once with the default 'all' value, then again with the 'flag' value specified in test_util.cc.
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6590
to look at the new patch set (#3).
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Add --redact=none flag option for specifying no redaction
It's still possible to specify no redaction via --redact='', but I was
only able to determine how to do that by reading the code. I think this
makes it a bit easier to use.
Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
---
M src/kudu/util/flags.cc
M src/kudu/util/logging.cc
2 files changed, 26 insertions(+), 41 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/6590/3
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Adar Dembo, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6590
to look at the new patch set (#4).
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Add --redact=none flag option for specifying no redaction
It's still possible to specify no redaction via --redact='', but I was
only able to determine how to do that by reading the code. I think this
makes it a bit easier to use.
Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
---
M src/kudu/util/flags.cc
1 file changed, 26 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/6590/4
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has uploaded a new patch set (#2).
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Add --redact=none flag option for specifying no redaction
It's still possible to specify no redaction via --redact='', but I was
only able to determine how to do that by reading the code. I think this
makes it a bit easier to use.
Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
---
M src/kudu/util/flags.cc
M src/kudu/util/logging.cc
2 files changed, 21 insertions(+), 41 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/6590/2
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Patch Set 3:
The change looks good to me, but can you check why the tests are failing? It looks like related.
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Patch Set 4: Code-Review+2
I'm curious as to how the diff between PS4 and PS3 fixed the test failures. As far as I can tell, all that changed is the location of the default initialization of the global booleans. Why did the first version break the tests, and how does this change fix them?
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Add --redact=none flag option for specifying no redaction
It's still possible to specify no redaction via --redact='', but I was
only able to determine how to do that by reading the code. I think this
makes it a bit easier to use.
Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Reviewed-on: http://gerrit.cloudera.org:8080/6590
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/util/flags.cc
1 file changed, 26 insertions(+), 38 deletions(-)
Approvals:
Hao Hao: Looks good to me, but someone else must approve
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Add --redact=none flag option for specifying no redaction
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Add --redact=none flag option for specifying no redaction
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6590/1/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
Line 136: } else if (redact_flags == "NONE" || redact_flags.empty()) {
> warning: don't use else after return [readability-else-after-return]
Done
Line 140: for (const auto& t : strings::Split(redact_flags, ",", strings::SkipEmpty())) {
> warning: the variable '__end' is copy-constructed from a const reference bu
no idea what this means
--
To view, visit http://gerrit.cloudera.org:8080/6590
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I06f9d1ab08baaac02359e3ab29f8741bd1f4ff7d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes