Danek Duvall wrote: > You've already put back a fix for bug 4483. This needs to be a new bug.
Really? You can't commit two changes via the same bug? I've been doing it wrong for so many years. :-) Okay. I've open bug #4592 for this: http://defect.opensolaris.org/bz/show_bug.cgi?id=4592 > >> Is this what we want here, or should we start from a fixed list? > > It might be useful to have empty categories, just to show that things will > eventually be placed there. If you want to support them, then I'd add an > option to the script that takes the empty categories as arguments. Talking to Frank Ludolph on this, he's fine with just having the ones found when doing this dynamic package classification. >> 3/ All packages in "opensolaris.org" that should have been of >> classification "System/X11 (System)" were previously written >> as "System/X11". > > I thought we got rid of the duplicate "System" on purpose. Don't forget this is "going the other way". I.e. we are now generating the files that the GUI Package Manager needs, and those still need to have some category values in brackets as part of the sub-category value, because it can't handle two sub-categories with the same name in two different categories. In other words: System/Databases -> System/Databases (System) System/Libraries -> System/Libraries (System) System/Localizations -> System/Localizations (System) System/X11 -> System/X11 (System) >> http://cr.opensolaris.org/~richb/pkg-4483-v2/ > > gen_os_files.py: > > - line 86: "dictory" Fixed. > > - line 92: I assume this is chopping off "classification"? Please split > on whitespace first, then on "/": > > line.split(None, 1)[1].split("/") > > for instance. Maybe you need an rstrip() in there, too, if line isn't > stripped already coming in to the method. So changed. I also rstrip()'ed the line coming into this method. > > - line 93: What happens if there is no slash? It would have failed. I've added a specific check for two tokens. If this isn't found, then an error message is written to stderr, and we just return. > > - line 98: You should explain why you're making these adjustments, as on > the face of it, they're pretty silly. I've added a comment. > > - line 114: spurious close-paren? Yes. Fixed. > > - line 132: "empty category" -> "empty subcategory"? Yes. Fixed. > > - line 135: I might just return here, rather than having the else clause. Agreed. So changed. This was there because the code that's now in do_classification was inline in the parent routine when I originally wrote it. > > - line 142, 145: Don't get in the habit of using non-specific except > clauses. You probably want to catch ValueError specifically. Okay. So changed. With Python 3, non-specific except clauses won't be allowed either. > > - line 223: This might be better off as a single triple-quoted string, > like in client.py. Okay. So changed. > > - line 314: "subcategoryr"? Fixed. > > - line 328: This strikes me as being a bit dangerous -- it's going to > pick up a lot of files that aren't import files, Yes but hopefully it handles them correctly and ignores the ones that are inappropriate. > and it's going to pick > up a lot of import files that aren't relevant to any given build. It does check to make sure it uses the one with the latest build number. If you have a suggestion for a better approach, then I'd like to hear about it. Don't forget that this: 1/ was initially a one hour hack to fix the files for the GUI Package Manager classification. Since then it seems to have taken on a life of its own. No doubt I'll be ARC'ing it soon. 2/ is hopefully going to be replaced in the next release with the GUI Package Manager code getting the classification data from the package manifests. New webrev at: http://cr.opensolaris.org/~richb/pkg-4592-v1/ Thanks! _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
