Adar Dembo has posted comments on this change. Change subject: [build-support] IWYU build configuration for Jenkins ......................................................................
Patch Set 3: (7 comments) http://gerrit.cloudera.org:8080/#/c/7750/3/build-support/iwyu/iwyu_tool.py File build-support/iwyu/iwyu_tool.py: So this is totally unmodified, then? http://gerrit.cloudera.org:8080/#/c/7750/3/build-support/jenkins/build-and-test.sh File build-support/jenkins/build-and-test.sh: Line 197: elif [ "$BUILD_TYPE" = "IWYU" ]; then Shouldn't this set USE_CLANG=1? Or does that not matter? PS3, Line 242: # Create empty test logs or else Jenkins fails to archive artifacts, which : # results in the build failing. : mkdir -p Testing/Temporary : mkdir -p $TEST_LOGDIR We have a number of these strewn around the script; can we just do this once however early it makes sense? Line 248: file_list_tmp=$(git show --format='' --name-only | grep -E '\.(c|cc|h)$') Maybe this should follow the style of 'make ilint', which looks for the last upstream commit and includes all commits since it. Look at build-support/lint.sh. PS3, Line 256: if [ -z "$IWYU_FILE_LIST" ]; then : # Nothing to do: declare success. : exit 0 : fi Maybe do this on file_list_tmp to short circuit faster? Line 267: --mapping_file=$IWYU_ARGS/gtest.imp \ Nit: after glog Line 274: IWYU_FILTERED_LOG=$TEST_LOGDIR/iwyu-filtered.log I'd prefer either: 1) Running awk as part of the iwyu pipeline so there's just the one filtered log at the end, or 2) Renaming the log files to iwyu-unfiltered.log and iwyu.log, so the log developers would want to see is the shorter one. -- To view, visit http://gerrit.cloudera.org:8080/7750 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I45f2e8529891dd7da18c9fa6383e0c41e6b4182e Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
