[EMAIL PROTECTED] wrote: > 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 > > Ok, I took the second suggestion. new webrev here: http://cr.opensolaris.org/~bpytlik/ips-4114-v3/
Brock _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
