Code review comment for lp:~lifeless/bzr/subunit

Revision history for this message
Matt Nordhoff (mnordhoff) wrote :

Matt's tiny nitpicks of the day:

25 === modified file 'bzrlib/osutils.py'

42 + if type(object) == str:

Types should be compared with "is".

71 === modified file 'bzrlib/tests/__init__.py'

156 + # pythons this goes sufficiently wrong that it is a bad idea. (
157 + # specifically a built in file with encoding 'UTF-8' will still try
158 + # to encode using ascii.

Where's the ")"? :D

198 +def _clever_some_str(value):
199 + try:
200 + return str(value)
201 + except:

Blanket except? That's scary.

290 + @staticmethod
291 + def _do_known_failure(self, result, e):

299 + @staticmethod
300 def _do_not_applicable(self, result, e):

308 + @staticmethod
309 + def _do_unsupported_or_skip(self, result, e):

I should probably read the testtools docs or something, but... Static methods that take "self"?

597 === modified file 'bzrlib/tests/blackbox/test_debug.py'

606 + log = u"".join(self.getDetails()['log'].iter_text())

It's a bit unpleasant to have that pasted all over the test suite. Though I guess it isn't duplicated *that* many times, given the number of tests...

« Back to merge proposal