Re: [pkg-discuss] Code Review for 5321 package manager dependencies tab could show installation status

Mon, 12 Oct 2009 08:13:33 -0700 

Looks good to me.

JR

Padraig O'Briain wrote:
I have respun the webrev, http://cr.opensolaris.org/~padraig/ips-5321-v8/, to address your comments. 

Padraig

On 10/12/09 11:33, jmr wrote:
Padraig ran and tested it, looks fine though I think the status icon and status text should be beside each other, seems odd having them at opposite ends of the line. 

Few comments on the code:
Should be using enums in here:
   def __setting_from_cache(self, pkg_stem):
You might not get states set here and so at 3084 will fail. I would suggest, reordering the line so icon and status info appear at the end, then if states is not set just don't show an icon and status text. 

    3038 +        def __set_dependencies_text(self, info, dep_info):
    3039 +                names = []
    3040 +                versions = []
    3041 +                states = None
3042 + if dep_info != None and len(dep_info.get(0)) >= 0: 
    3043 +                        states = dep_info[0]


JR

Padraig O'Briain wrote:
I have respun the http://cr.opensolaris.org/~padraig/ips-5321-v7/ to display the version, build and branch information in the same way as in the General Info tab. 


Padraig

On 09/25/09 13:42, Joanmarie Diggs wrote:

WFM. Thanks.
--joanie

On Fri, 2009-09-25 at 10:51 +0100, Padraig O'Briain wrote:

I have respun the webrev
http://cr.opensolaris.org/~padraig/ips-5321-v6/.
I believe that it fixes the problem Joanie identified.

Padraig
On 09/18/09 06:18, Joanmarie Diggs wrote: 
Hey Padraig.

I found one more traceback:

        Traceback (most recent call last):
File "/usr/bin/packagemanager", line 3171, in __update_package_info 
            self.__set_dependencies_text(local_info, dep_info)
File "/usr/bin/packagemanager", line 2980, in __set_dependencies_text if states and states[i].state == api.PackageInfo.INSTALLED: 
        IndexError: list index out of range
This one I came across when examining certain packages I have in a local 
repo. I've not figured out exactly what triggers it. For instance,
having switched views to my local repo, selecting SUNWcairo does not
trigger the error, but for some reason selecting SUNWcairomm does.

$ pkg list -av SUNWcairo
FMRI STATE UFIX pkg://mynga/[email protected],5.11-0.122:20090916T051212Z known ---- pkg://opensolaris.org/[email protected],5.11-0.122:20090828T200631Z installed u--- 

$ pkg list -av SUNWcairomm
FMRI STATE UFIX pkg://mynga/[email protected],5.11-0.122:20090916T055116Z known ---- pkg://opensolaris.org/[email protected],5.11-0.122:20090828T200634Z installed u--- 

<shrugs>

--joanie

On Thu, 2009-09-17 at 23:16 -0400, Joanmarie Diggs wrote:

Hey Padraig.
First, I must say that the new layout is awesome. Makes it much easier 
to understand what's present and absent.

I did find an occurrence where things seem to fail, however. When I
select a locale for OpenOffice (e.g. openoffice-hu), I get the following 
traceback:

Traceback (most recent call last):
File "/usr/bin/packagemanager", line 3171, in __update_package_info 
    self.__set_dependencies_text(local_info, dep_info)
File "/usr/bin/packagemanager", line 2950, in __set_dependencies_text 
    if dep_info != None or len(dep_info.get(0)) >= 0:
AttributeError: 'NoneType' object has no attribute 'get'

--joanie

On Thu, 2009-09-17 at 10:39 +0100, Padraig O'Briain wrote:

The webrev, http://cr.opensolaris.org/~padraig/ips-5321-v4/, fixes
5321 package manager dependencies tab could show installation status
This webrev changes the layout of the dependencies panel to separate the package name and version and list whether it is installed or not installed. 

Padraig
_______________________________________________
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

Reply via email to