Hi Sanjay,

Nit:

1230-1233:
The if/else is unneeded. Line 1231 is all that's needed:
pkgkey = pkgcomp[0].rsplit('/')[-1]

I was momentarily concerned about ambiguous package names: e.g., if a 
user passes in

some/package/program
other/package/program

The key for each would be "program" and only the last one listed would 
get installed. However, I doubt we have any other distro_const or AI 
code to handle such a thing yet either, so you're not introducing a new 
problem (and this specific case of the problem goes away when we can 
batch install everything and skip the manual parsing of the package names).

Nit:
1245-11247:
Would be better as:
if not pkgdict:
     return


On 03/24/10 11:37 AM, sanjay nadkarni (Laptop) wrote:
> The code review with the change is now available at:
> http://cr.opensolaris.org/bugfix_15132
>
> -Sanjay
>
>
> On 03/24/10 09:52 AM, sanjay nadkarni wrote:
>> On 03/24/10 07:47 AM, Dave Miner wrote:
>>>
>>>>> 1249-52: I'd think you could turn this into simply
>>>>>
>>>>> pkgs = " ".join(order.values())
>>>>>
>>>> That will not work, since order is a list of keys not a dictionary.
>>>>
>>>
>>> Yeah, missed that.  Fine then.
>>>
>>> Dave
>> I have a fix that addresses the primary concern that you expressed 
>> i.e. will keys work if the pkg is expressed as a complete FMRI and I 
>> will be posting the webrev soon.
>>
>> The supported formats can be any of the following. I am using entire 
>> as an example here.
>> entire
>> entire at ent...@0.5.11,5.11-0.134
>> entire at 0.5.11,5.11-0.134:20100302T023003Z
>> pkg://pkg.opensolaris.org/entire at 0.5.11,5.11-0.134
>> pkg://pkg.opensolaris.org/entire at 0.5.11,5.11-0.134:20100302T023003Z
>>
>> -Sanjay
>>
>>
>>
>> -Sanjay
>>
>> _______________________________________________
>> 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

Reply via email to