Looks good to me.
Padraig
On 11/20/09 14:41, Michal Pryc wrote:
Padraig,
http://cr.opensolaris.org/~migi/12297_check_for_updates_ver3
A few more comments:
1) As you have changed "Check or Updates" to "Updates" in menu and
toolbar should you change the text in preferences dialog?
Changed.
2) Why are you returing True in __on_preferencesdialog_delete_event()?
This prevents the dialog to be destroyed, we are using show/hide
functions and once you pres escape when the dialog is up and running
and then re-open the PM will core.
3) What bug are the changes in repository.py fixing?
http://defect.opensolaris.org/bz/show_bug.cgi?id=12571
With plural form if more publishers are affected.
4) What is the purpose of pkg_b in __after_plan_information in
installupdate.py?
Removed pkg_b.
5) How do I expand the description column to see all the description
in the confirmation dialog?
Mouse over the right corner of the "Description" column header.
6) Does this change have any impact on Web Install?
Not that I am aware of. We are passing the "confirmation_list"
variable to the InstallUpdate which by default is None (for all the
other InstallUpdate consumers than Package Manager) and if it is None,
then InstallUpdate is behaving as it was before this change, so It's
going directly to the self.__proceed_with_stages() call.
Michal
Padraig
On 11/20/09 12:02, Michal Pryc wrote:
Padraig,
New webrev:
http://cr.opensolaris.org/~migi/12297_check_for_updates_ver2/
Coments inline.
Padraig O'Briain wrote:
Some initial comments:
packagemanager-preferences.schemas.in:
There is a full stop at line 198 which I believe should not be there.
Removed.
gui_pylintrc: Change from 5000 to 6000 seems excessive. Would 5100 do?
Changed.
Should Ann Kanodia be added to the list of names in the glade file?
For sure. Added.
Are we having separate checkboxes for Check for Updates and
Install/Update?
I do not see it in the wireframe and I thought we were going to use
one checkbox for both.
It's not in the wireframes, but it's something we do want. I will
ping Ann to add this to the wireframes.
There was some discussion of changing "Check for Updates" to
"Updates". I do not see a bug for this and you do not seem to be
doing this in this webrev.
Changed to Updates with tooltip "Checks if updates are available".
In Remove confirmation dialog should the title be "Remove
Confirmation"?
In Update all confirmation should the title be "Update All
Confirmation"?
Renamed.
Should the packages in this list be sorted by name?
Fixed, now it's sortable by all three things (name, publisher and
description), by default it's sorted by name.
I find the existence of the expander in this dialog very strange.
Should this be just a label which is not shown if there are no
packages to be removed?
The expander it's in the wireframes, I have added spacing between it
and I hope it's looking better.
Should there be more space between the label and the treeview below
it?
Added 5px.
The dialog does not seem to be resizable. Should it be? I might
want to see all the description.
The dialog is not resizable, otherwise the expanders will behave
incorrectly. The column with description is resizable, so you can
make it wider (by resizing the column, not dialog).
best
Michal
On 11/19/09 15:53, Michal Pryc wrote:
Hello,
I have prepared webrev for the bug:
12108 Add "Do not prompt" checkbox on confirmation dialogs
http://defect.opensolaris.org/bz/show_bug.cgi?id=12108
Which also adds one part which was described in the comment #1 for
bug:
12297 Change Update All to Check for Updates
http://defect.opensolaris.org/bz/show_bug.cgi?id=12297
The webrev is available at:
http://cr.opensolaris.org/~migi/12297_check_for_updates_ver1/
It implements new confirmation dialog for Package Manager together
with adding hooks to the preferences dialogs and gconf keys.
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
