Code review comment for lp:~knittl/bzr/cat-signature

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

cmd_cat_signature should be indented to 4 spaces, not 2 as per our guidelines, and there should be some more spacing between options.

We probably also want the output encoding to be exact. The content should be 7-bit safe, but just in case.

I'm not sure that @display_command actually makes sense. It means that we'll ignore stuff like EPIPE, I'd probably take it off, but it isn't a big deal.

Something like:

+class cmd_cat_signature(Command):
+ __doc__ = """Show signature of a revision's testament"""
+
+ hidden = True
+ takes_options = ['revision']
+ takes_args = ['branch?']
+ encoding = 'exact'
+
+ def run(self, branch=u'.', revision=None):

46
47 === added file 'bzrlib/tests/blackbox/test_cat_signature.py'
48 --- bzrlib/tests/blackbox/test_cat_signature.py 1970-01-01 00:00:00 +0000
49 +++ bzrlib/tests/blackbox/test_cat_signature.py 2010-09-23 16:27:46 +0000
50 @@ -0,0 +1,58 @@
51 +# Copyright (C) 2007-2010 Canonical Ltd

Copyright for this file should be only 2010

But the tests themselves seem fine to me. And the actual functional code seems good.

review: Needs Fixing

« Back to merge proposal