Matthew Jacobs has posted comments on this change. Change subject: IMPALA-3225: Add script to push from gerrit to ASF ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/3468/1/bin/push_to_asf.py File bin/push_to_asf.py: PS1, Line 89: pass > This can be removed, yeah? Done Line 104: OPTIONS = None > Seems weird to stick this one constant midway down the file. Can it be move Done PS1, Line 115: print >>sys.stderr, "No remote named 'apache'. Please set one up, for example with: " : print >>sys.stderr, " git remote add apache", APACHE_REPO > This is probably just copy/pasted from the Kudu script, but using successiv Valid questions/concerns, though I don't think it's worth spending too much time investing in improving the formatting of the output for the moment because it's not clear how long this will live. I'll add a TODO at the top of this to clean up the print statements if you're OK with that for now. If this proves to be something we want to keep for the longer term, we should invest more in future proofing this. Line 219: print Colors.GREEN + ("%d commit(s) need to be pushed from Gerrit to ASF:" % len(commits)) + Colors.RESET > Long lines here and elsewhere. Done PS1, Line 253: + > Minor: Python concatenates adjacent strings within braces automatically, so Done -- To view, visit http://gerrit.cloudera.org:8080/3468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I15de939acc4b08a8f01511fffcb6ab1743d09c01 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Matthew Jacobs <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
