Looks good to me. Padraig
On 01/19/09 16:23, Michal Pryc wrote: > 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
