Code review comment for lp:~gz/python-oops-tools/dbsummaries_utf8_encode_891186

Revision history for this message
Graham Binns (gmb) wrote :

Hi Martin,

This change looks good to me. A couple of cosmetic comments, though. The branch is good to land once you've fixed these:

[1]

61 + python_oops = {'id': 'OOPS-1234S101', 'reporter': 'edge',
62 + 'type': 'Exception', 'value': u'a unicode char (\xa7)',
63 + 'time': datetime(2008, 1, 13, 23, 14, 23, 00, utc)}

The formatting here's a bit dense. Dicts should be one element per line:

        python_oops = {
            'id': 'OOPS-1234S101',
            'reporter': 'edge',
            'type': 'Exception',
            'value': u'a unicode char (\xa7)',
            'time': datetime(2008, 1, 13, 23, 14, 23, 00, utc),
            }

74 + def test_renderHTML_with_unicode_data(self):
75 + # Using cStringIO here to simulate a file object open without an
76 + # encoding specified.

This comment should state the expected behaviour rather than saying what
the test does. (Saves lazy developers from reading the code unless the
thing breaks.)

review: Approve (code)

« Back to merge proposal