Code review comment for lp:~jr/bzr/bzr-gpgme

Revision history for this message
Jonathan Riddell (jr) wrote :

> 113 + if from_revno is None or to_revno is None:
> 114 + raise errors.BzrCommandError('Cannot verify a range of \
> 115 + non-revision-history revisions')
> As far as I know this isn't possible - perhaps this should be an AssertionError rather than a BzrCommandError?

This happens if you specify versions from a merged revision, bzr verify -r 3.1.1..3.1.2 which isn't supported by this code
The code comes from builtins.cmd_re_sign()

> For consistency we've been adding --directory/-d option to most commands; it would be nice to support that to "bzr verify" as well.

Added.

> I think the command name is perhaps a bit generic, perhaps it should be "bzr verify-signatures", with "verify" as one of its current aliases.

Done.

> I'm unsure about supporting a --acceptable-keys argument to verify.

I trust lots of people to have digital keys that match their ID, but
it doesn't mean I trust them to commit to my code branch, so there
does need to be a way to specify which keys are acceptable.

> We should watch out with displaying the signature status for each
  revision in "bzr log"; it has the potential to significantly slow
  down log if more people start signing their revisions.

That's why it's an option which isn't on by default.

Jonathan

« Back to merge proposal