Adar Dembo has posted comments on this change. Change subject: Add a script to push gerrit branches to ASF ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/1723/2/build-support/push_to_asf.py File build-support/push_to_asf.py: You've docstring'ed nearly all of the functions; would you mind adding docstring to the few remaining functions, for completeness? Line 46: # ANSI color codes. Fancy! Line 85: if GERRIT_HOST not in url: We could check for something more specific like: ssh://<username>@gerrit.cloudera.org:29418/kudu Right? Or can people use gerrit with non-ssh access too? Thought you needed ssh to push to gerrit. Line 157: apache_sha[:8], gerrit_sha[:8]) Could let "git rev-parse --short" decide how much of the SHA to print. My understanding is that "how much" depends on how many objects you've got in the repo (i.e. long enough to disambiguate properly). Line 171: ['git', 'log', '--color', '-n1', '--oneline', sha]).strip() Decompose this into a small function like rev_list() , rev_parse() or get_committer_email()? Alternatively, it might be more readable if you went in the opposite direction and converted all of those into inline calls to subprocess.check_output(). Either way, more consistency would be nice. -- To view, visit http://gerrit.cloudera.org:8080/1723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I60eefe99c2ba3bbd9c9df15f307d9ec277c2ce29 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-HasComments: Yes
