Danek Duvall wrote: > On Mon, Nov 24, 2008 at 01:59:43AM -0600, Shawn Walker wrote: > >> Greetings, >> >> The following webrev contains changes for the following RFEs: >> 3011 new depot index page desired >> 3014 revamped depot bui (web ui) desired >> >> webrev: >> http://cr.opensolaris.org/~swalker/pkg-3014/
Issues addressed are omitted from my reply. > server/api.py: > - In general, this looks nice, though I'd question why this is being done > as a separate module, rather than massaging the underlying modules to > do the right thing. Adding a layer to fix problems in lower layers is > wrong. I suppose it's just a matter of running out of time? I'm not sure I understand you here. My whole purpose in creating the server API was two-fold: 1) provide a stable interface to the request, catalog, and configuration of the server that unknown clients (i.e. random user) can use in templates 2) abstract away and hide various parts of the api that shouldn't be used by anyone but the server internally (i.e. destroy_catalog, etc.) 3) prevent major internal changes from breaking template consumers or allow us to completely change the web back-end (i.e. BaseHTTP to CherryPy) transparently I thought these were the same reasons that moved us to create a client API. I never made these changes to avoid bugs in the underlying API. Remember that the templates provided by the depot are completely customisable, and it is intended that anyone can write their own set of depot facing pages using the API provided instead of relying on ours. Now one thing that I could see is that it's called pkg.server.api, but it's really an API for clients getting information from the server, it isn't to manipulate the server itself. As such, I might have picked the wrong name. Perhaps pkg.server.client.api? > server/api_errors.py: > > - line 47: Wouldn't the old and new URLs be useful / interesting data to > store here? Perhaps not yet. They would only seem to be useful at the point that the exception occurred. At that point, the new URL is exception_object.data and the old url is cherrypy.url(). It would be easy enough to add later if a need presents itself. > face.py: > Why is there a trailing backslash after all "%>"? Is that simply to avoid > having zillions of empty lines? Yes; the '/' consumes the newline. > Is there a standard way for editors to recognize a file as a mako template? None that I've found. I did find a vim syntax thing for Mako, but I've had to do "set ft=mako" each time I edit a file. It's annoying. Note that the '-*- coding: utf-8 -*-' line at the beginning tells Mako what the encoding of the file is -- it isn't for vim. Cheers, -- Shawn Walker _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
