On Sun, 2013-05-05 at 22:13 +0200, Niels Thykier wrote: > On 2013-01-13 16:38, Niels Thykier wrote: > > I have written a series of patches to reduce the size of britney.py. > > In total, these patches removes 274 lines from britney.py[1]. There > > are some refactoring patches interleaved with this that I thought > > would be useful to do, but I can take them out if preferred. > > > > Reviews greatly appreciated,
Thanks for these, and sorry for not looking over them properly earlier. On the whole, they look good; thanks. At least one patch would have been easier to review if a "-w" version had been provided as well. :) A few queries / comments: Firstly, I'm assuming that the resulting britney produces the same result from the testsuite. + same_source is an opt to avoid "load global". Could we avoid using shorthand here? I'm assuming "optimisation" and something along the lines of "the overhead of forcing a lookup of a function from the global namespace"; the expansion of the latter is probably too far in the other direction though. :( + This method returns the full dependency tree for the package + `pkg`, inside the `arch` architecture for the suite `suite` + flatterned as an iterable. I was going to grumble about the quoting style here, but then realised it's already in use in other parts of the code; I still don't really like it though. We appear to have at least three different quoting styles in use in various parts of the code. :-( + If given an iterable it returns a filtered iterator, otherwise it + returns a function to generate filtered iterators. The latter is + useful if the same filter has to be (re-)used on multiple + iterators that are not known on before hand. "beforehand" is a single word; there's another occurrence in the same file. + The tree (or grapH) is returned as an iterable of (package, arch) + tuples and the iterable will contain (`pkg`, `arch`) if it is + available on that architecture. s/grapH/graph/ Regards, Adam -- To UNSUBSCRIBE, email to debian-release-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org Archive: http://lists.debian.org/1368283744.7990.40.ca...@jacala.jungle.funky-badger.org
