Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/9715 )
Change subject: [build] Reduce the chances of a build abort corrupting the build ...................................................................... Patch Set 1: (6 comments) I don't know whether to be surprised or impressed that you chose to continue using shell instead of rewriting the script in Python. Anyway, looks fine as-is. http://gerrit.cloudera.org:8080/#/c/9715/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9715/1//COMMIT_MSG@14 PS1, Line 14: Also introduces a “kudu-download-complete” file that : is used similarly to the patch level file, but indicates if : the build was interrupted during the download/patch : process. It seems like kudu-download-complete has the same semantics as patchlevel: 1. Verified before the download step. 2. Written out after the patching steps. Given that, what value does it add on top of patchlevel? http://gerrit.cloudera.org:8080/#/c/9715/1//COMMIT_MSG@20 PS1, Line 20: rapid-json Nit: rapidjson http://gerrit.cloudera.org:8080/#/c/9715/1/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: http://gerrit.cloudera.org:8080/#/c/9715/1/thirdparty/download-thirdparty.sh@35 PS1, Line 35: kudu-download-complete > nit: all those file are kind of auxiliary, any maybe it's better to name th I don't think it's as important for files that are placed within the dependency's source directory (see how we treat "patchlevel" files). http://gerrit.cloudera.org:8080/#/c/9715/1/thirdparty/download-thirdparty.sh@128 PS1, Line 128: # local PATCH_COMMANDS A list of patch commands is expected last Errant comment? http://gerrit.cloudera.org:8080/#/c/9715/1/thirdparty/download-thirdparty.sh@140 PS1, Line 140: touch kudu-download-complete Shouldn't this be written after the download step? Otherwise doesn't it behave identically to patchlevel? http://gerrit.cloudera.org:8080/#/c/9715/1/thirdparty/download-thirdparty.sh@218 PS1, Line 218: $CMAKE_PATCHES \ Does this need to be double quoted? -- To view, visit http://gerrit.cloudera.org:8080/9715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I34b5065ebaf09503b0dc6daf11b962f5a41f7d76 Gerrit-Change-Number: 9715 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Mon, 19 Mar 2018 18:24:32 +0000 Gerrit-HasComments: Yes
