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

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

I think you're on the right track with this; here are some more comments. It would be great if somebody else (with more GPG experience) could also comment.

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?

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

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. We currently already have "bzr check" and to me, as a non-native speaker, these two words mean roughly the same thing in this context.

I'm unsure about supporting a --acceptable-keys argument to verify. I think it would be sufficient for verify to just check that the signatures match what is actually in the repository, leaving the issue of who to trust up to the user for the time being. I don't see myself using --acceptable-keys because I usually only care about the signature on a particular revision (not the history leading up to that point) or there are too many revisions and thus too many keys to specify (bzr has > 200 contributors, it's not practical to specify all their key ids on the command line).

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.

« Back to merge proposal