Merge lp://qastaging/~bilalakhtar/brasero/unity-launcher-integration into lp://qastaging/~ubuntu-desktop/brasero/ubuntu

Proposed by Bilal Akhtar
Status: Merged
Merged at revision: 110
Proposed branch: lp://qastaging/~bilalakhtar/brasero/unity-launcher-integration
Merge into: lp://qastaging/~ubuntu-desktop/brasero/ubuntu
Diff against target: 153 lines (+123/-1)
4 files modified
debian/changelog (+7/-0)
debian/patches/013_unity_launcher_progress.patch (+113/-0)
debian/patches/series (+1/-0)
debian/rules (+2/-1)
To merge this branch: bzr merge lp://qastaging/~bilalakhtar/brasero/unity-launcher-integration
Reviewer Review Type Date Requested Status
Michael Vogt (community) Approve
Review via email: mp+68940@code.qastaging.launchpad.net

Description of the change

Display burn progress in brasero's Unity launcher icon. Patch has been forwarded to GNOME Bugzilla @ https://bugzilla.gnome.org/show_bug.cgi?id=655161 .

To post a comment you must log in.
111. By Bilal Akhtar

Add --enable-unity flag to rules

Revision history for this message
Jeremy BĂ­cha (jbicha) wrote :

Don't you need a build-depends, on something like libunity-dev?

Revision history for this message
Michael Vogt (mvo) wrote :

Thanks for your branch!

When trying to build this I get:
...
checking for UNITY... no
configure: error: Package requirements (unity) were not met:

No package 'unity' found

So it appears there is a build-dependency missing on "libunity-dev". I added that now and with that it builds fines. And works fine too :)

review: Approve
Revision history for this message
Michael Vogt (mvo) wrote :

Only one suggestion, it would probably be better to test priv->launcher_entry before using it:

E.g.
++#ifdef HAVE_UNITY
++ unity_launcher_entry_set_progress_visible(priv->launcher_entry, TRUE);
++#endif

could become:
++#ifdef HAVE_UNITY
++ if (priv->launcher_entry)
++ unity_launcher_entry_set_progress_visible(priv->launcher_entry, TRUE);
++#endif

to ensure that if:
++ priv->launcher_entry = unity_launcher_entry_get_for_desktop_id("brasero.desktop");
returns NULL the application does not crash. In the NULL case it should probably also
log a huge warning.

Revision history for this message
Michael Vogt (mvo) wrote :

The NULL check is not critical at this point, so I uploaded the current version.

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

to all changes: