Keith,
Thank you for the review. I saw if part (line 912) this morning and
said to myself that I need to fix it. So I have using your suggestion
with the append. I also modified how the key width is calculated. The
same part of code used to loop through the returned temporary dictionary
to figure out the criteria key widths. Now I have the get_criteria_info
return both the temporary list (tdict) and the criteria key width saving
a loop.
The slightly modified webrev
http://cr.opensolaris.org/~johnfisc/list-output-13550/
Thanks,
John
Keith Mitchell wrote:
> Lines 912, 916 and 917 shouldn't be needed now. sdict will always have
> the name key.
>
> Sidenote: line 915 is redundant, and 913-914 could be reduced to:
>
> sdict[name].append(tdict)
>
>
> - Keith
>
> 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
