Code review comment for lp:~jml/testrepository/show-failures-incrementally-613152

Revision history for this message
Robert Collins (lifeless) wrote :

+ def _make_result(self, repo, evaluator):
+ if self.ui.options.list:
+ return evaluator

this two things seem disconnected; perhaps rather than evaluator you should say list_result or something. I think something originally together has been split out far enough that the parameter needs a better name.

+Wildcard = _Wildcard()

perhaps
wildcard = Wildcard()

would be nicer. Many things (like str, object, etc) are lowercase for instances in Python.

+ def _output_run(self, run_id):

def _output_summary

 - I think.

+ return ''.join([
+ self.sep1,
+ '%s: %s\n' % (label, test.id()),
+ self.sep2,
+ error_text,
+ ])

Looks like a lurking UnicodeDecodeError to me; we either need to make this unicode always or manually encode error. One way to test that would be to throw a mixed encoding, non-ascii test outcome at it.

Would you be kind enough to do these tweaks? then its definitely gtg.

review: Approve

« Back to merge proposal