Code review comment for lp://qastaging/~mterry/aptdaemon/progress-tweaks

Am Dienstag, den 05.06.2012, 17:29 +0000 schrieb Michael Terry:
> Michael Terry has proposed merging lp:~mterry/aptdaemon/progress-tweaks into lp:aptdaemon.
>
> Requested reviews:
> Aptdaemon Developers (aptdaemon-developers)
>
> For more details, see:
> https://code.launchpad.net/~mterry/aptdaemon/progress-tweaks/+merge/108789
>
> This branch implements some of the progress dialog changes from https://wiki.ubuntu.com/SoftwareUpdates
>
> Specifically:
> 1) Drops the in-dialog icon

The cute little animation... :)

> 2) Updates a couple of the header labels' wording

Be careful: ROLE_COMMIT_PACKAGES can be an installation, removal, update
or downgrade of packages. Even at the same time. So you cannot reduce
the name to installing updates only.

If you only want to update packages you should use the update packages
call in update-manager.

Why do you want to add the three points? At first this should be
consistent with all other enums. Secondly it should be consistent with
the other apps from the GNOME desktop.

If you want to set a custom label we should better add support for this
to the dialog: Perhaps adding a message_format or label parameter to
__init__()?

> 5) Adds an API call to ask not to update the title bar (the consumer can then set it to something custom)

I would prefer if the title would be set to the role automatically if it
is empty instead of introducing another setter.

if not self.get_title():
    self.set_title(role)

The role of a transaction cannot be changed and so will always be the
same.

Or is this too much magic?

> 6) Adds an API call to ask not to show the details expander

It would be nice to have a hidden option for update-manager to allow
showing the details (gnome-tweaks?) if you plan to remove them by
default. It is quite popular by users to watch the downloads - it seems
to give them some feeling of control. Alternatively the terminal can be
hidden by the terminal parameter separately.

Thanks,

Sebastian