On Wed, Oct 22, 2008 at 03:15:25PM -0700, [EMAIL PROTECTED] wrote:
> On Wed, Oct 22, 2008 at 03:03:55PM -0700, Brock Pytlik wrote:
> > [EMAIL PROTECTED] wrote:
> > > In general, this looks okay.  However, I'm concerned about the way we're
> > > iterating through the manifest at line 683 in apy.py.  We've added a
> > > dictionary, indexed by the type of the action, to the manifest object.
> > > We should be able to get this information without iterating over every
> > > action in the manifest.
> > >
> > > -j
> > >   
> > Good catch. Using this method certainly simplifies the code.
> > New webrev (mostly posted for this change):
> > http://cr.opensolaris.org/~bpytlik/ips-4114-v2/
> 
> Perhaps I'm missing something obvious, but wouldn't it be simpler to
> re-write these list comprehensions as:
> 
>       [ a.attrs.get(a.key_attr)
>         for a
>         in mfst.gen_actions_by_type(<type>)
>       ]
> 
> -j

It may also be worthwile to add whatever method you choose as a function
to manifest.  That way if we change how the actions_by_type data is
stored, we'll have less duplicated code to modify.

-j

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

Reply via email to