Code review comment for lp:~vila/bzr/1249732-acceptable-keys-from-config

John A Meinel (jameinel) wrote :

I think the patch got read backwards. When we parse commandline args, the
parser turns everything into Unicode objects. So this was just trying to
turn them back into str objects.
I like the idea of testing a list rather than just a single entry. There is
also a typo in NEWS (bazzar.conf)
Otherwise it gets an approve from me. (I can't vote on LP from my phone)

On Nov 11, 2013 1:33 AM, "Richard Wilbur" <email address hidden> wrote:

> Review: Needs Fixing
> Upon further consideration, it looks like it would be useful to add a test
> of a list of acceptable_keys. If Unicode occurs in E-mail addresses and/or
> domain names it would also be useful to add a test of with an
> acceptable_keys list including one or more containing Unicode characters.
> --
> Your team Bazaar Codereview Subscribers is subscribed to branch lp:bzr.

« Back to merge proposal