On 2011-11-01 07:04, Adam D. Barratt wrote: > On Thu, 2011-10-27 at 21:52 +0200, Niels Thykier wrote: >> The major part of the development in the lab-refactor branch should be >> done now, so I intend to merge it into master. The branch has touched >> major parts of Lintian and breaks a lot of stuff. Obviously, I would >> like a thorough review of it before I hit merge - particuarly of >> reporting/* as we have no test-suite coverage here. >> >> I would like to see the merge done some time next week followed by an >> upload. However if you would like more time to review the branch, >> please let me know how long you need. > > I started having a look over the branch at the weekend; unfortunately a > combination of being busy with other (mostly work-related) stuff and not > feeling great means it hasn't been as useful as I'd hoped so far. > Nevertheless I thought it was worth recording my thoughts so far. >
Thanks for the comments so far. :) > * As discussed on IRC, it may be worth considering whether we want to > s/lab_// in a number of method names. > Yes, I do not feel strongly either way. As I recall I added it because some methods had the "lab" marker and others didn't. Btw, I presume you also want "s/_lab//" (i.e. create_lab => create)? > * How much of reporting/ has been tested with the new lab code? I > realise we don't have formal test coverage for it but from memory > getting a minimal setup working is fairly simple. > Not a lot to be honest. I have tested html_reports with the standard log file from lintian.d.o and an empty lab. Beyond that, "perl -Ilib -c $script" is about the testing it has received. > * I noticed there's code intended for v10 lab format backward > compatibility. Given that the patch also updates the lab format, how > much has that compatibility code been tested? > The only compatibility code is "lab_exists" and "remove_lab", because else the API cannot remove an existing v10 Lab. I just tested it on my v10 lab. It fails to open the lab and can remove the lab. I did consider adding a migration script that could migrate a v10 Lab to a v11, but I believe it will very limited in usefulness. Particularly, on the first upload after merging we will probably need a full run to pick up the packages not checked right now. > * Finally whilst having an initial run through the code I made a number > of tweaks and reminder comments, mostly related to documentation. I'm > afraid it's not nicely split up but I've attached a copy of the changes > in the hope that they're useful. > Thank you. :) > Regards, > > Adam ~Niels -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
