Rhythmbox

Merge lp://staging/~larsu/rhythmbox/add-traditional-menubar into lp://staging/~ubuntu-desktop/rhythmbox/ubuntu

  1. add-traditional-menubar
  2. Merge into ubuntu
Proposed by Lars Karlitski
Status: Merged
Approved by: Sebastien Bacher
Approved revision: 222
Merged at revision: 214
Proposed branch: lp://staging/~larsu/rhythmbox/add-traditional-menubar
Merge into: lp://staging/~ubuntu-desktop/rhythmbox/ubuntu
Diff against target: 581 lines (+493/-6)
7 files modified
debian/changelog (+37/-0)
debian/control (+4/-3)
debian/control.in (+1/-0)
debian/patches/make-shuffle-repeat-proper-toggle-actions.patch (+116/-0)
debian/patches/restore-traditional-menubar.patch (+330/-0)
debian/patches/series (+2/-0)
debian/rules (+3/-3)
To merge this branch: bzr merge lp://staging/~larsu/rhythmbox/add-traditional-menubar
Reviewer Review Type Date Requested Status
Sebastien Bacher Approve
Review via email: mp+208772@code.staging.launchpad.net

Commit message

Add restore-traditional-menubar.patch

Description of the change

Add restore-traditional-menubar.patch

To post a comment you must log in.
Revision history for this message
Sebastien Bacher (seb128) wrote :

Looks mostly good, some small issues:

- repeat/shuffle are displayed unactive

- there is an empty "tools" menu (not a new issue iirc)

- the volume actions don't seem to do anything visible?

review: Needs Fixing
lp://staging/~larsu/rhythmbox/add-traditional-menubar updated
219. By Lars Karlitski

Correct package version

220. By Lars Karlitski

restore-menubars: move "Plugins…" to the Tools menu

So that the tools menu isn't empty when no plugin installs menu items there.

221. By Lars Karlitski

Add make-shuffle-repeat-proper-toggle-actions.patch

Revision history for this message
Lars Karlitski (larsu) wrote :

> Looks mostly good, some small issues:
>
> - repeat/shuffle are displayed unactive

This was a problem with the actions themselves. They had an unnecessary boolean parameter. I've added a patch that fixes them. It's a separate patch because it has better chances at getting accepted upstream than the menubar one (which I still hope to get in).

> - there is an empty "tools" menu (not a new issue iirc)

Fixed by moving the "Plugins" menu item from Edit to Tools as suggested by Matthew.

> - the volume actions don't seem to do anything visible?

As discussed on IRC, they do change the apps volume but are pretty useless. We decided to leave them in though.

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks, looks good, one remaining nitpick, the "tools" menu doesn't get a separator before the plugins... entry if there are other items listed

lp://staging/~larsu/rhythmbox/add-traditional-menubar updated
222. By Lars Karlitski

restore-menubar: wrap Plugins menu item in a section

Revision history for this message
Lars Karlitski (larsu) wrote :

> Thanks, looks good, one remaining nitpick, the "tools" menu doesn't get a
> separator before the plugins... entry if there are other items listed

Thanks. Fixed in r222

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches