David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/11128 )
Change subject: IMPALA-7399: Add script in lib/python to generate junit XML. ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/11128/5/lib/python/impala_py_lib/jenkins/generate_junitxml.py File lib/python/impala_py_lib/jenkins/generate_junitxml.py: http://gerrit.cloudera.org:8080/#/c/11128/5/lib/python/impala_py_lib/jenkins/generate_junitxml.py@49 PS5, Line 49: elif os.path.isfile(file_or_string): > This is funky, but you want "os.path.exists" here. It breaks magic things l Done http://gerrit.cloudera.org:8080/#/c/11128/5/lib/python/impala_py_lib/jenkins/generate_junitxml.py@89 PS5, Line 89: """[Required] Specific build step or child script being run. > I tend to find the [Required] as a bit redundant given the existing usage: Done http://gerrit.cloudera.org:8080/#/c/11128/5/lib/python/impala_py_lib/jenkins/generate_junitxml.py@99 PS5, Line 99: either a plain string, or the path to a file print out.""") > nit: "either a plain string or a path" I'm not sure what happened here. This was some kind of copy/paste error, and I was sure I'd changed this per your previous comment. Sigh. Really sorry about that. http://gerrit.cloudera.org:8080/#/c/11128/5/lib/python/impala_py_lib/jenkins/generate_junitxml.py@104 PS5, Line 104: either a string, or the path to a file.""") > same nit: I don't think you need the comma here. Done -- To view, visit http://gerrit.cloudera.org:8080/11128 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If6024d74075ea69b8ee20d1fc3cc9c1ff821ba5b Gerrit-Change-Number: 11128 Gerrit-PatchSet: 5 Gerrit-Owner: David Knupp <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Thu, 09 Aug 2018 17:31:53 +0000 Gerrit-HasComments: Yes
