Adar Dembo has posted comments on this change.

Change subject: Fix and improve code coverage
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/2448/2/README.adoc
File README.adoc:

Line 242
Is it no longer true (that lcov produces better HTML)? Or just irrelevant?


Line 236: use the tool like `lcov` or `llvm-cov gcov` to visualize the results. 
For example, using
Nit: I think the previous "use a tool like..." sounds more correct.


Line 237: gcovr:
Hmm, this is a little confusing w.r.t. the previous sentence, which doesn't 
mention gcovr. Can you reword the text so it's a little more clear what's going 
on?


http://gerrit.cloudera.org:8080/#/c/2448/2/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:

Line 306:       -r $SOURCE_ROOT/src \
Note that BUILD_ROOT/src is where you'd find the generated PB source. Should we 
include that into coverage reports?


http://gerrit.cloudera.org:8080/#/c/2448/2/src/kudu/util/logging.cc
File src/kudu/util/logging.cc:

Line 141:     needs_flush = false;
This must be single-threaded, otherwise I'd expect TSAN would complain about 
non-atomic access to needs_flush?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If0273fe095d8f160f0068ec421fceb54eda588de
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

Reply via email to