Hao Hao has posted comments on this change.

Change subject: Combine redaction flag for log and flags into one.
......................................................................


Patch Set 8:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/6112/7/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:

PS7, Line 130:     kudu::should_redact_log = false;
             :     return true;
             :   }
             : 
> You can use some of the handy functions from gutil/map-util.h for this, lik
Done


PS7, Line 140:     return true;
             :   }
             : 
             :   
> If you use the DEFINE_validator() macro, you won't need to worry about savi
Done


PS7, Line 509:     retval += f.name;
             :     retval += 
> Style: we use underscores when naming local variables, not camel case. Only
Done


http://gerrit.cloudera.org:8080/#/c/6112/7/src/kudu/util/flags.h
File src/kudu/util/flags.h:

PS7, Line 73: 
            : 
            : 
            : 
            : 
            : 
            : 
            : 
            : 
> This was more complicated than I anticipated, so let's scale it back. Maybe
Done


http://gerrit.cloudera.org:8080/#/c/6112/3/src/kudu/util/logging.h
File src/kudu/util/logging.h:

Line 62: #define KUDU_SHOULD_REDACT() (kudu::should_redact_log && 
kudu::tls_redact_user_data)
> The value cannot be changed; the redaction gflag isn't tagged as 'runtime'.
Done


http://gerrit.cloudera.org:8080/#/c/6112/7/src/kudu/util/logging.h
File src/kudu/util/logging.h:

Line 32: 
////////////////////////////////////////////////////////////////////////////////
> warning: using declarations in the global namespace in headers are prohibit
Done


Line 32: 
////////////////////////////////////////////////////////////////////////////////
> These are legit; you shouldn't put "using" statements in headers.
Done


Line 32: 
////////////////////////////////////////////////////////////////////////////////
> warning: using decl 'ShouldRedact' is unused [misc-unused-using-decls]
Done


Line 33: // Redaction support
> warning: using decl 'RedactionUnit' is unused [misc-unused-using-decls]
Done


Line 33: // Redaction support
> warning: using declarations in the global namespace in headers are prohibit
Done


Line 58: 
> Nit: restore this indentation?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia49ae439824351df6c87291b0828173c69ab31b0
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to