On 10/25/13 01:31 PM, Yiteng Zhang wrote:
Ok, a new webrev can be seen at
Sorry that I dont add a comment to the new webrev.
Please let me know your comments.
Looking at this it seems a cleaner approach would be to collect the exceptions in a simple list first and then only raise the exception if anything is in this list. Look at how InvalidOptionErrors is used, basically you can do the same thing here.
This way you don't need an additional variable to keep track if you actually had at least one exception and you only create the ExpiredCertificates exception when needed. That's a small change.
relying on the fact that publishers are always in the correct sequence is sketchy. It's probably better to create a dictionary (publisher: [uri1, uri2, ...]) and then cycle through all the publishers in this list.
There is some work needed here to have these messages properly localized. The problem is that the length of the fields is not fixed in a way that we can format this message appropriately since the field names might have different lengths in different languages. The localization folks don't want to have any space formatting in the message strings for that reason so I suggest we do the following:
You should be able to generate this like that:
msg = "%s\n" % publisher
msg += " %s:\n" % _("Origin URI")
msg += " %s\n" % uri
Please break the lines like this:
[pkg-discuss] Re: Review request 15507548 cert validation needs to validate all certificates before raising e