[email protected] wrote: > On Tue, Oct 20, 2009 at 10:25:58AM -0700, Danek Duvall wrote: > > > http://cr.opensolaris.org/~swalker/pkg-cat-p3/ > > > transport.py: > > > > - line 376: Why this test? > > The client has to compute the succesful requests if failures occurred. > Otherwise, all requests are assumed to have succeeded.
I guess what I was seeing was that success would be computed correctly by the filter regardless of whether failedreqs was empty or not, and having flist be the empty list rather than None wouldn't change the way the rest of the code worked (and if you ever chose to iterate over it, you wouldn't have to test to see if it were None). > > - line 377: success = [ x for x in flist if x not in failedreqs ] ? > > What's wrong with filter? It changes in py3k, returning an iterator instead of a list. Between compatibility with the future and consistency in our code, I'd rather see the comprehension, particularly since it's really not a hard one to read. > > - line 414: > > > > if failedreqs and failures: > > failures = [ x for for x in failures if x.request in failedreqs > > ] > > tfailurex = tx.TransportFailures() > > tfailurex += failures > > raise tfailurex > > The TransportFailures object coalesces re-occuring errors. I don't > believe that the += is going to work. Ah. TransportFailures objects have a special append() method, rather than inheriting from list objects? Fair enough. The two tests could still be improved. Danek _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
