Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11377 )
Change subject: KUDU-2411. Add scripts to build binaries for testing use ...................................................................... Patch Set 4: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py File build-support/relocate_binaries_for_mini_cluster.py: http://gerrit.cloudera.org:8080/#/c/11377/3/build-support/relocate_binaries_for_mini_cluster.py@217 PS3, Line 217: : def copy_file(src, dest): : > I'm not really concerned about this from a performance perspective because To me it wasn't so much about performance but about more user-friendly flow of the script (I alluded to this in the second sentence in my suggestion). As a user I like working with scripts that work like this: 1. Check a bunch of stuff up front without mutating any on-disk state, erroring out if something is missing. 2. Do all the work. This gives me confidence that if I see an error in step 1, none of my on-disk state has changed. In this script, that would mean moving check_for_chrpath() to just before prep_artifact_dirs(). All that said, I appreciate your point about keeping it close to where chrpath is actually used, and I don't feel strongly about it. -- To view, visit http://gerrit.cloudera.org:8080/11377 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8b8f90cfe80f6830177bf6ea9e0711eb0d034f75 Gerrit-Change-Number: 11377 Gerrit-PatchSet: 4 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Fri, 04 Jan 2019 22:31:03 +0000 Gerrit-HasComments: Yes
