Code review comment for lp:~javier.collado/checkbox/bug990075-2
Revision history for this message
| Marc Tardif (cr3) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
f33e652
(Get the code!)
So, you had a few comments that I will try to address individually in this comment:
1. About the duplicate tests, I was able to reproduce the problem. I reintroduced some of the code in the JobStore class in revno 1447 and I was then unable to reproduce the problem. Fixed!
2. About sorting dicts, I was also able to reproduce the problem. I specified the key parameter to the sorted() function in revno 1445 and that exception no longer appeared in the log. As for the mysterious revno 1446, that fixed another unrelated exception that no longer appears in the log either. Double fixed!
3. About the reordering, it seems that the motivation for the error message when the whitelist does not honour dependencies is to encourage developers to use the whitelist as the authority. In other words, the most common use case will be to use the ordering specified in the whitelist. So, we could potentially improve the ordering in the Resolver class but I don't think it's really worthwhile. What do you think?
All the changes have been ported from the bug990075-3_trunk to the bug990075-3_0.13 branch. So, you should be able to test once again.