[Dev] review of BuildingChandlerParcels

[Bryan Stearns]

Ted Leung wrote:

> Building Chandler Parcels <http://svn.osafoundation.org/chandler/ 
> branches/Chandler_0.6/chandler/distrib/docs/ 
> BuildingChandlerParcels.html> is a revision of this year's PyCon  
> paper <http://wiki.osafoundation.org/bin/view/Documentation/ 
> BuildingChandlerParcels> to reflect all the changes that we've made  
> in 0.6.  [...] I'd love it if a few people could take some time to 
> review it and  suggest improvements.

Hi Ted,

This is the first time I've looked in any detail at the Feeds parcel, so 
some of these comments apply to the doc as a whole not just to your 
recent revision of the doc), and/or to the parcel as implemented, ... 
anyway, do with them what you will.

...Bryan

- While FeedChannel is a great name, it's a singular name for a 
collection;I think it might be clearer to find a new name for 
FeedChannel that reflects that it's a collection of things. (it's 
already a little confusing that Item Collections are Items too.) Also, 
"Behind the scenes: ItemCollections" seems out of place where it is, and 
could probably move up to the earlier FeedChannel discussion.

- It'd be great to find a different name for the FeedChannel's "Update" 
method, since it's being introduced within inches of the schema "update" 
method (and while FeedChannel's use of that word is more understandable 
than schema's use of 'update' to create new instances, changing 
FeedChannel's use seems simpler at this point). As one not afraid of 
excessively-long method names, I'll suggest checkForNewFeedItems() :-).

- The explanation for FeedItem starts by saying it contains four 
attributes, though the immediately-previous source lists six.

- "Behind the scenes: Repository" says that "our Feed Item" has a 
SuperKind called ListCollection - actually, it's that's FeedChannel. 
(This paragraph might be a good place to mention more explicitly that 
collections are Items too... or maybe this is the place for "Behind the 
scenes: ItemCollections".)

- The "Adding RSS Item and RSS Channel: Extending Chandler's Schema" 
starts by saying that we're not extending Chandler's schema yet. This 
cries out to be separated out, maybe as a "Basics of the Chandler Data 
Model" subsection analogous to the existing "Basics of the Chandler UI" 
section?
 
- There seems to be some naming confusion in & after the "Connecting the 
Data to the User Interface" section: you mention RSSChannel and RSSItem 
objects - which seem to be the old names of FeedChannel and FeedItem? 
Also, NewFeedChannelEvent/onNewFeedChannelEvent are sometimes referred 
to as NewFeed_s_ChannelEvent/onNewFeed_s_ChannelEvent in text as well. 
In "Behind the scenes: CPIA", you bolden "ContentModel items", which 
probably oughta be just ContentItems, or just "content model items".

- The detail view in Feeds is not based on Attribute Editors :-(; I'd 
hoped someone would fix this in 0.6, but never got to it myself. Even if 
you don't switch the example to use AEs, you might want to make it use 
the detail-view's mechanisms for some of the boilerplate stuff (for 
example, in "Displaying an Item's Detail View", your example calls 
functions called pair(), label(), and field(), though it doesn't define 
them; the detail view parcel provides makeArea(), makeLabel(), and 
makeEditor(), as well as makeSpacer(). You might look at scripts.py for 
a more-current example of a detail view that isn't part of the detail 
view parcel.

- Minor nits: The displayNames should all be _(u"whatever") - some don't 
have the _(); "retreive" should be "retrieve" in a couple of places; 
FeedUpdateTaskClass needn't have "Class" in its class name;

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.osafoundation.org/pipermail/dev/attachments/20051201/7d99d2ad/attachment.html