Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11235 )

Change subject: IMPALA-7399: Remove third-party dependencies from junit xml 
script
......................................................................


Patch Set 5:

I don't know if it's bad form, but I figured I'd just push a proposal here via 
PS5 instead of figuring out how to use gist or cutting and pasting an entire 
file of python. For comparison, doing string interpolation is about the same 
size as the original (160 lines), and beats generating the XML via code. It's 
about the same as the original. I didn't test it with Jenkins (or diff), so I'm 
sure it'll take another few iterations to figure that out.

I'm definitely not tied to it; I just wanted to see if I buy my own arguments 
here.

As a reader of the PS4 code, I got confused by the functions. I think if we go 
that route, it would be clearer to do:

init:
  self.make_root()
  self.attach_testsuite()
  self.attach_testcase()

i.e., make it clear that we're constituting this thing.

I do worry that I'm bike-shedding here. Feel free to bat me away, and we can 
move on :)


--
To view, visit http://gerrit.cloudera.org:8080/11235
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I958ee0d8420b6a4197aaf0a7e0538a566332ea97
Gerrit-Change-Number: 11235
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: Nithya Janarthanan <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Comment-Date: Thu, 16 Aug 2018 00:26:31 +0000
Gerrit-HasComments: No

Reply via email to