Code review comment for lp:~krause/bzr/1037108

Revision history for this message
John A Meinel (jameinel) wrote :

Since this is an issue for 'log', I think either bzrlib/tests/test_log.py or bzrlib/tests/blackbox/test_log.py would be good places. (I prefer the former as a white-box test, vs the later as a whole-command test.)

All of those tests should pass on all platforms, and you can just run those tests with:
  bzr selftest -s bt.test_log -s bb.test_log

The change itself seems ok, but maybe incomplete. It seems that the real problem is that we are not decoding result[1]. So it comes in as a UTF-8 string (8-bit str with high bits set.) When what we need is a python Unicode string. It is also good to have the format strings be unicode.

So a fix like:

unicode_result = result[1].decode('utf-8') # Can we assume utf8?
if result[0] == gpg.SIGNATURE_VALID:
  return u'valid signature from {0}'.format(unicode_result)
...

etc.

review: Needs Fixing

« Back to merge proposal