Sorry, I meant to say "please" - I do appreciate the code reviews :-)
On Thu, 2010-09-23 at 09:22 +1200, Tim Foster wrote: > Hi All, > > Brock's already taken a look at this, but I'm still looking for one more > reviewer on the changes which incorporate Brock's feedback. > > cheers, > tim > > On Tue, 2010-09-21 at 13:06 +1200, Tim Foster wrote: > > On Mon, 2010-09-20 at 15:51 -0700, Brock Pytlik wrote: > > > > http://cr.opensolaris.org/~timf/info-classification-webrev > > > > Thanks for taking a look! > > > > > lint/opensolaris.py: > > > line 95: I wonder if it isn't worth displaying some soft of message > > > here. The error on line 149 is useful, but then I'm left wondering what > > > was wrong w/ the file. > > > > This would be a rare case, since the default data file isn't user > > modifiable, but I've improved the error messaging here, and increased > > the priority of the lint message in the case of a missing or broken data > > file. > > > > > 191: typo in category > > > 211: the error message says "does contain" should it be "does not > > > contain"? > > > > Yep. > > > > > 211-214: This message might be more useful if it clarified which bit > > > isn't in the classification file, and maybe even showed what the valid > > > sections were (if it was an invalid section) or what the valid > > > categories were (if it was an invalid category)? > > > > Sure. I've added a few more messages, which are a lot more verbose (I'm > > using different sample files to produce the errors below) > > > > ERROR opensolaris.manifest003.4 info.classification value > > org.opensolaris.category.2008:Noodles/Networking does not contain one of > > the valid sections Development, Desktop (GNOME), Drivers, System, > > Applications, Web Services, Meta Packages for > > pkg://opensolaris.org/pkglint/[email protected],5.11-0.141:20100604T143737Z. > > > > ERROR opensolaris.manifest003.5 Invalid info.classification value for > > pkg://opensolaris.org/pkglint/[email protected],5.11-0.141:20100604T143737Z: > > data file /tmp/opensolaris.org.sections does not have a 'category' key > > for section Drivers. > > > > ERROR opensolaris.manifest003.6 info.classification attribute in > > pkg://opensolaris.org/pkglint/[email protected],5.11-0.141:20100604T143737Z > > does not contain one of the values defined for the section Drivers: > > Display,Media,Networking,Other Peripherals,Ports,Storage > > from /tmp/opensolaris.org.sections > > > > This almost feels like overkill for checking the value of a single > > attribute, but at least there'll be no excuse for not fixing it now. > > > > > General comment: > > > Marking the variables to indicate whether they contain reference data > > > (ie, data for the file) or the data being validated (ie, the info from > > > the action) would probably make this easier to read. I got confused > > > about the difference between components and categories on my first read > > > through. > > > > I've improved the naming of the variables here too and have updated the > > webrev in place: > > > > http://cr.opensolaris.org/~timf/info-classification-webrev > > > > cheers, > > tim > > > > > > _______________________________________________ > > pkg-discuss mailing list > > [email protected] > > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss > > > _______________________________________________ > pkg-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/pkg-discuss _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
