On Sun, Jun 01, 2008 at 10:27:13PM -0500, Shawn Walker wrote: > > - line 72: you're not actually doing a version check here. Is there a > > I hadn't thought about checking the version, but I really should. The > current code will only work with 3.0.3 but less than 3.1.0 (not yet > released at last check). > > > reason that you're bothering to explicitly wrap the import? > > Yes, it was suggested by Krister.
Okay (I skipped most of that subthread, since I was away). It's not clear to me that this is useful -- on a system that hasn't been hacked to bits, it'll be there (otherwise SUNWipkg won't even install). I'll want this to go away, though, certainly once cherrypy gets properly integrated. > > - on line 17, you return a status code, but on line 191, you raise an > > exception. What's the difference? > > ilne 17? I'm not sure what line you're referring to. However, I tried > to keep most of this code the same it was before. So if it is > returning one now that's probably why. Can you clarify? Errr. Not sure about my original intended line number, but line 79 will do. Basically, I can't tell why in some cases you're raising exceptions to make the server return a particular HTTP status code and why in some cases you're doing so via a return value. Is there a useful difference between the two methods? If not, shouldn't they all be done the same way? > > - Do you think it would be feasible / cleaner to decorate each of the > > operations methods with attributes designating what operation and > > version they represent, and then build the vop table out of those > > decorations, rather than the method names? My lack of understanding of > > what's going on in default() might preclude that. > > It sounds like it would be possible. However, a pointer to the actual > method is still needed for CherryPy's dispatch table. > > You can find more information about how its default dispatcher works here: > http://www.cherrypy.org/wiki/CherryPyTutorial#Findingthecorrectobject > > and here: > http://www.cherrypy.org/wiki/CherryPyTutorial#Partialmatchesandthedefaultmethod > > Of course a custom dispatcher could be written which would certainly > change things. For simplicity's sake, I was trying to stick with the > default one provided. Okay, I'll take a look, thanks. I'm certainly not going to hold your wad back for this sort of exploration. > > SUNWpython-cherrypy/Makefile: > > > > - I wonder if perhaps doing the tarball download higher up, in the main > > "make install" wouldn't be better -- so that it'll work out of the box, > > and you could integrate it into "make link" and so on. > > Eventually cherrypy will go away and not be part of the build, so I'm > not certain. Yeah, but we don't have a timeframe on that yet, do we? Seems like we could be in the current situation for a while. > > - Why is the CDDL here? > > I was copying the approach from the other files. > > I have removed it and the Sun copyright notice. Please correct me if I'm > wrong. That should be right -- the license and copyright are for the bits delivered by the package, not for the copyright file itself. > > - line 34, 35: This needs to point to us/Sun, since we're the distributor > > (vendor) here. > > Ah. I changed the vendor, what about the email? I've removed it for now. Email probably should be whatever it is for SUNWipkg. > I have posted an updated webrev here: > http://cr.opensolaris.org/~swalker/pkg-depot-9/ > > ...and a udiff of the changes from the last webrev here: > http://cr.opensolaris.org/~swalker/pkg-depot-9/raw_files/pkg-depot-9.patch Thanks. I'll look over them now. Danek _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
