Dan Price wrote:
> On Wed 30 Jul 2008 at 05:28PM, Shawn Walker wrote:
>> Shawn Walker wrote:
>>> I'm sending out a completely new review request for this wad as I have 
>>> changed it significantly since the last webrev I sent out.  The old 
>>> versions are still available for comparison.
>>>
>>> The following webrev includes proposed fixes for the following bugs:
>>>
>>>    1324 RSS / Atom feeds of repository updates
>> original webrev:
>> http://cr.opensolaris.org/~swalker/pkg-1324/
>>
>> previous webrev:
>> http://cr.opensolaris.org/~swalker/pkg-1324-4/
>>
>> new webrev:
>> http://cr.opensolaris.org/~swalker/pkg-1324-5/
>>
>> changes from last webrev:
>> http://cr.opensolaris.org/~swalker/pkg-1324-5/pkg-1324-4-5.patch
> 
> Can you please update the 'pkg info' code to use your new
> bytes_to_str() routine?  (And anything else that uses such measures?)
> It's OK to defer that to a followup putback, but it does need a bug
> filed against it.

I filed:
2761 ensure client has consistent output for units of measure

> server/face.py: if head() is really supposed to return a
> common header for all pages, then I wouldn't think that the
> auto-feed-thingy should show up on every page... should it?

My idea was that visiting any BUI page would show the RSS feed 
subscription icon, so it was intentional.  It makes little difference to 
me if it only shows up on the index (status) page.

> It would sort-of seem to lock us in to one feed per depot.
> Maybe I misunderstand how the code is supposed to work.

I don't think it does, but I do intend the index page at the very least 
to have it there so that users visiting a depot's main page will get a 
subscription icon.

> feed.py: 31: please distill this into a bug/RFE (if not done
> already) then just omit 31-34.

It's Stephen's comment, so I don't know that I can accurately distill 
it.  However, I made a wild guess and filed:

2762 package metadata should support storing entity entity origin 
information

The lines have been removed.

> 200: XXX -- can you add that, or file a bug?

Filed:
2763 pkg.catalog should have a function to split entries into 
appropriate fields


> repositoryconfig.py: 129: why this change?

pylint warning fix about definition of attribute outside of __init__.

> repository.py: this may be a stupid question, but what's the
> logic behind wiring the feed to a specific server op rather than
> a face.py handler?

No idea, it's what Stephen had originally, and it doesn't matter to me.

>  From what I can tell, each element of the feed will
> contain a link to an 'info' URI, which causes the server to cough up
> text/plain info about a given package.  It seems odd for that to be a
> non-HTML response, to me.

It's a non-HTML response because I don't want to do extensive HTML or 
page coding until I have a real template system in place.  Since the 
output can change whenever we want, I figured it could be done later as 
further enhancements to the BUI are made.

> What's the impact on browsers of emitting a table with 10000 rows?

Not much in my experience.  I've sent far more HTML than that to user 
agents.

Long-term, we shouldn't be display all entries in the catalog on the 
front page.

-- 
Shawn Walker
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to