I have updated the webrev. The new one is
http://cr.opensolaris.org/~padraig/ips-5375-v6/.
More detailed comments below.
Padraig
On 01/21/09 10:22, jmr wrote:
> Looks good with Shawn's suggested text mods +1.
>
> Few minor niggles:
>
> Following should be removed:
>
> 283 + #auths =
> self.img.gen_authorities(inc_disabled=True)
>
>
> 304 + print name
> 305 + print str(l)
> 306 + print a["disabled"]
>
I have removed these.
>
> Prefer use of All Cap variables ("constants" by convention in python)
> here to the numbers as its clearer what each column is referring to
> and less error prone:
> 396 500 self.old_modify_url =
> model.get_value(itr, 2)
> 501 + self.old_modify_ssl_key =
> model.get_value(itr, 3)
> 502 + self.old_modify_ssl_cert =
> model.get_value(itr, 4)
> 397 503 self.old_modify_preferred =
> model.get_value(itr, 1)
> 504 + self.mirror_list.clear()
> 505 + mirrors = model.get_value(itr, 5)
>
>
> 731 + def __add_mirror(self):
> :
> 742 + mirrors =
> model.get_value(itr, 5)
>
>
> 773 + mirrors =
> model.get_value(itr, 5)
>
>
I have added constants in enumerations.py for these.
> Looks like we using the image object directly here, this needs to be
> captured in the new Authority API, so we can replace this call. Can
> you update the bug and put in a comment to that effect here.
> 765 + def __delete_mirror(self, name, mirror):
> 766 + try:
> 767 + self.img.del_mirror(name, mirror)
>
I have updated
http://defect.opensolaris.org/bz/show_bug.cgi?id=5871
> JR
>
> Padraig O'Briain wrote:
>> I have updated the webrev, at
>> http://cr.opensolaris.org/~padraig/ips-5375-v5/, so that it applies
>> after Michal's recent commit.
>>
>> Padraig
>>
>>
>> On 01/19/09 12:10, Padraig O'Briain wrote:
>>> I have corrected some errors in the previous webrev. The latest one
>>> is at
>>> http://cr.opensolaris.org/~padraig/ips-5375-v4/.
>>>
>>> Padraig
>>>
>>> On 01/13/09 09:29, Padraig O'Briain wrote:
>>>> I have reworked the webrev,
>>>> http://cr.opensolaris.org/~padraig/ips-5375-v3/, to address Shawn's
>>>> comments.
>>>>
>>>> Padraig
>>>>
>>>> On 01/08/09 17:10, Shawn Walker wrote:
>>>>
>>>>> Padraig O'Briain wrote:
>>>>>
>>>>>> The webrev http://cr.opensolaris.org/~padraig/ips-5375-v2/ fixes
>>>>>> Bug 5375 PackageManager repository management should include
>>>>>> security info
>>>>>> RFE 5595 packagemanager does not provide a way to manage /
>>>>>> specify authority mirror
>>>>>>
>>>>>> When a repository is being added and its name starts with "https"
>>>>>> a dialog is displayed allowing the user to specify SSL key and
>>>>>> certificate.
>>>>>>
>>>>>> In "Modify Repository" dialog, mirrors can be added or removed
>>>>>> and SSL key and certificate can be changed.
>>>>>>
>>>>> data/packagemanager.glade:
>>>>> lines 2149-2120: suggested rewording, fixes:
>>>>>
>>>>> If this repository requires a SSL Key and Certificate, or it has
>>>>> mirrors, add a new repository first and then modify to add this
>>>>> information.
>>>>>
>>>>> lines 2919-2922: suggested rewording, fixes:
>>>>>
>>>>> If this repository requires a SSL Key and Certificate, please
>>>>> enter it into the SSL section.
>>>>>
>>>>> If you want to alter the list of mirrors for this repository, use
>>>>> the mirrors section.
>>>>>
>>>>> line 3346: an opacity of .999.... something? Really? Why not
>>>>> just 1.0?
>>>>>
>>>>> lines 3363-3365:
>>>>>
>>>>> This repository requires a SSL Key and SSL certificate. Enter the
>>>>> location of these files in the spaces provided, or use the Browse
>>>>> buttons to provide their location.
>>>>>
>>>>> gui/modules/repository.py:
>>>>> All of your _() i18n strings are using the '%s' style of
>>>>> substitution instead of the '%(var_name)s' style that is needed
>>>>> when you have more than one substitution.
>>>>>
>>>>> Cheers,
>>>>>
>>>> _______________________________________________
>>>> pkg-discuss mailing list
>>>> [email protected]
>>>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> pkg-discuss mailing list
>> [email protected]
>> http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
>>
>
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
