Shawn Walker wrote:
> Brock Pytlik wrote:
>> http://cr.opensolaris.org/~bpytlik/ips-1929-v3/
>>
>> Resynced to gate and updated per Danek's comments. Looking for one 
>> more reviewer before I put back.
>
> client.py:
>   line 180: maybe all_versions would be a better variable name than 
> collapse?
>
k
>   line 229: s/[hv]=/[hv] =/
>
fixed
>   line 233: Some comments explaining the sorting and what affect each 
> statement has might be nice.  Such as, "fmris with the preferred 
> authority are returned first", etc.  Some newlines before and after 
> this nested function might also be helpful for readability.
Does this comment satisfy the need?
                        # This method is necessary because fmri.__cmp__ does
                        # not provide the desired ordering. It uses the same
                        # ordering on package names as fmri.__cmp__ but it
                        # reverse sorts on version, so that 98 comes 
before 97.
                        # Also, authorities are taken into account so that
                        # preferred authorities come before others. Finally,
                        # authorties are presented in alphabetical order.
>
>   line 245: Are you sure you don't want get_authority() instead of 
> get_authority_str()?  I ask because the comment for 
> get_authority_str() indicates that it's really only for code that 
> writes out manifests to disk.
changed
>
> pkg.1.txt:
>   line 223: s/FMRIS. With/FMRIs.  With/
fixed
>
> Otherwise, looks fine.
>
> Cheers,

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to