Jack, Thanks for the review!!
John Jack Schwartz wrote: > LGTM. > > Thanks, > Jack > > John Fischer wrote: >> Jack and Keith, >> >> I have updated the webrev for this defect. >> >> http://cr.opensolaris.org/~johnfisc/list-output-13550/ >> >> to match the discussion. >> >> Thanks, >> >> John >> >> >> John Fischer wrote: >>> Keith, >>> >>> I have moved that line up and remove the if statement. I have >>> tested this on onol-inst.sfbay and it works for both the case >>> that revealed the bug and all other cases. >>> >>> Thanks, >>> >>> John >>> >>> >>> Keith Mitchell wrote: >>>> Why not start off the beginning of the for loop with "sdict[name] = []" >>>> >>>> Then the if/else at lines 911-916 isn't needed. >>>> >>>> - Keith >>>> >>>> John Fischer wrote: >>>>> All, >>>>> >>>>> Here is a simple code review for installadm list subcommand. >>>>> The current code uses the else clause for a for loop. The >>>>> name within the else clause is not defined within that context. >>>>> Removing the else causes the code to be in the main 'for name' >>>>> loop which has the 'name' defined within that context. >>>>> >>>>> http://cr.opensolaris.org/~johnfisc/list-output-13550/ >>>>> >>>>> This issue only shows up when a manifest has no criteria which >>>>> is why I missed it within the first code drop. >>>>> >>>>> Thanks, >>>>> >>>>> John >>>>> _______________________________________________ >>>>> caiman-discuss mailing list >>>>> caiman-discuss at opensolaris.org >>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >
