LGTM Brock
Michal Pryc wrote: > Brock Pytlik wrote: >> Michal Pryc wrote: >>> Brock, >>> New webrev is at: >>> http://cr.opensolaris.org/~migi/08_12_2008_bug5645_5289_4341_5150/ >>> >>> I don't know why the pylint didn't catch the nit which you were >>> talking about (line 956) but this is corrected. >>> >>> Also I did clean up the unused static function >>> check_if_pkg_have_row_in_model() and removed "\" from the parts which >>> I've been changing and contained the "\" inside parens. So a little >>> bit more cleanup then the previous webrev. >>> > Brock, >> Great thanks for doing this. > Thanks for looking into the changes in the first place! > >> I haven't totally groked the changes made and why they offer such a >> substantial speed up, but it's clear they do. Thanks for adding the >> details to the bug, now at least I can see what's different. >> >> I'm fine with not moving the code in 1471-1510 into Api.py (or >> image.py). I suggest the middle ground of moving it to misc.py and >> pulling it out of both client.py and packagemanager.py. While I agree >> that robust stable interfaces are desired to solve these problems, I >> still don't think we should introduce more potential problems before we >> get those interfaces by copying the code. Is that agreeable? > That is fine with me. I've made the webrev with this and hopefully you > will give me +1 for it, so I will submit to the gate: > > http://cr.opensolaris.org/~migi/09_12_2008_bug5645_5289_4341_5150/ > > I did pylint the code as well I've check if everything works fine (pkg > list/pkg list -a/pkg list -af/pkg list -afv and packagemanager). I've > also ran the tests. _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
