Padraig O'Briain wrote: > I went through the webrev with Michal and have a few comments:
Padraig, Thanks, new webrev with changes: http://cr.opensolaris.org/~migi/dialog_rework_16_jan_2009_v2/ > 1) At line 1861 of packagemanager.py > self.update_all_proceed = None > should be > self.update_all_proceed = False Changed. > > 2) In installupdate.py at lines 128 and 129 > infobuffer.create("blue", foreground="black") > infobuffer.create("red", foreground="black") > > I believe that these lines should be removed. Also, the references in > the code to blue and red > should be removed. This was used when I was playing with colors, but then we've decided to have everything in black, so I did remove the "blue" and "red" together with all references. > 3) Line 352 is the only call to __g_exception_stage. The first two > arguments, can be removed. Changed. > 4) Line 526 is > > c = _("Downloaded ") + size_a_str + _(" of ") + size_b_str > > Should this be changed to something line > > c = _("Downloaded %(cur)d of %(tot)d" % \ > {'cur': size_a_str, > 'tot': size_b_str} Changed. > 5) Some functions contain the one line pass and some the one line return. > I think that they should all the the same/ Changed to return rather then pass. > 6) In __indexing_progress why are the lines 728 to 731 necessary? > If they are we need to find a more elegant way as sleeping until a > variable is set looks ugly. You are right. I have removed this code together with same ones close to line 475. It's not necessary as this situation will not occure. best Michal > > > > On 01/16/09 15:13, Michal Pryc wrote: >> I didn't clean up the packagemanager.py, so the new version is: >> >> http://cr.opensolaris.org/~migi/dialog_rework_16_jan_2009_v1 >> >> best >> Michal >> >> Michal Pryc wrote: >>> Hello, >>> The webrev is at: >>> >> >>> This webrev includes major rework for the install/remove/update dialogs. >>> More information can be found (internal site, if someone interested >>> please let me know and I will attach to the e-mail): >>> >>> Remove: >>> >>> http://xdesign.sfbay.sun.com/projects/solaris/subprojects/package_mngt/UI_specs/ui_spec_phase3/html-mockup/21_remove_dialog.htm >>> >>> >>> >>> Install/Update: >>> >>> http://xdesign.sfbay.sun.com/projects/solaris/subprojects/package_mngt/UI_specs/ui_spec_phase3/html-mockup/03_install_progress.htm >>> >>> >>> >>> Update All: >>> >>> http://xdesign.sfbay.sun.com/projects/solaris/subprojects/package_mngt/UI_specs/ui_spec_phase3/html-mockup/54_update_progress.htm >>> >>> >>> >>> Due to the nature of new dialog design it fix the followig bugs: >>> >>> "No confirmation after operation was completed" >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5231 >>> >>> "PM fails with a blank error message" >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=4848 >>> >>> "packagemanager should be showing more detailed index updating info" >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5089 >>> >>> "The error for InventoryException should be more informative." >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5449 >>> >>> "Child windows should be restricted to the workspace of the parent/main >>> window" >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5641 >>> >>> "package manager confirmation message incorrect" >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5320 >>> >> >> _______________________________________________ >> 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
