libappindicator

Merge lp://staging/~jjardon/libappindicator/fix-820080 into lp://staging/libappindicator

  1. fix-820080
  2. Merge into trunk
Proposed by Javier Jardón
Status: Rejected
Rejected by: Ted Gould
Proposed branch: lp://staging/~jjardon/libappindicator/fix-820080
Merge into: lp://staging/libappindicator
Diff against target: 82 lines (+7/-31)
1 file modified
src/app-indicator.c (+7/-31)
To merge this branch: bzr merge lp://staging/~jjardon/libappindicator/fix-820080
Reviewer Review Type Date Requested Status
Ted Gould (community) Disapprove
Andrea Cimitan Pending
Review via email: mp+76660@code.staging.launchpad.net

This proposal supersedes a proposal from 2011-09-21.

To post a comment you must log in.
Revision history for this message
Andrea Cimitan (cimi) wrote : Posted in a previous version of this proposal

I don't know which is the default coding style, but from the snippet I see you might want to remove the space between function names and their brackets

review: Approve
Revision history for this message
Ted Gould (ted) wrote : Posted in a previous version of this proposal

While I agree that this is the "right" way to do it, GIcon support has been broken in GTK+ for a while and was causing upstream GNOME maintainers to claim our work was shoddy.

https://bugzilla.gnome.org/show_bug.cgi?id=614800

So that's why we stopped using it.

review: Disapprove
Revision history for this message
Ted Gould (ted) wrote :

Sorry I took so long on this. We can't drop adding the "-panel" name extension on the icons as we want the fallback to look the same as the icon would look in the primary case. The "-panel" extension is used so that icon themes can theme the panel differently as they're not given anything like a StyleContext to determine their location otherwise.

review: Disapprove

Unmerged revisions

215. By Javier Jardón

Do not append "-panel" to the given icon name if it's missing.

As the -panel variant can not exist. Also, GtkStatusIcon
support fixed-sized icons nowadays.

Fix https://bugs.launchpad.net/libappindicator/+bug/820080

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