Looks good to me now, Thanks,
Darren. On 25/10/2011 15:12, Drew Fisher wrote: > Darren, > > Version 2 is here: > > full webrev: > https://cr.opensolaris.org/action/browse/caiman/drewfish/7091425_2/webrev/ > incremental: > https://cr.opensolaris.org/action/browse/caiman/drewfish/7091425_incr1/webrev-1-2-incremental/ > > Thanks for looking! > > -Drew > > On 10/25/11 7:07 AM, Darren Kenny wrote: >> On 25/10/2011 14:00, Drew Fisher wrote: >>> Darren, >>> >>> On 10/25/11 3:13 AM, Darren Kenny wrote: >>>> Hi Drew, >>>> >>>> I took at look at this, in general all looking good, but I do have some >>>> questions/comments: >>>> >>>> - Generally >>>> >>>> I wonder if the name 'create_child_slice' doesn't convey the fact that >>>> all existing slices will be deleted well enough. >>>> >>>> I don't have any specific suggestion and >>>> "create_slice_replacing_all_others" is probably a bit verbose ;) >>> If you feel strongly about the name, I could maybe do: >>> create_single_slice() or something like that? >> I do think there needs to be some thing that make it obvious that it's >> going to replace all existing slices - not just creating a new slice. >> >>>> - test_shadow_list.py >>>> >>>> Would it be worth adding a test to verify that it's the only slice >>>> created after the call. >>> Sure, I could add that. I'll get a code review up to you shortly. >> Thanks, >> >> Darren. _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
