libindicator

Merge lp://qastaging/~3v1n0/libindicator/scroll-event-support into lp://qastaging/libindicator/0.4

scroll-event-support
Merge into trunk

Proposed by Marco Trevisan (Treviño) on 2011-01-26

Status:

Merged

Merged at revision:

395

Proposed branch:

lp://qastaging/~3v1n0/libindicator/scroll-event-support

Merge into:

lp://qastaging/libindicator/0.4

Diff against target:

89 lines (+28/-4)

3 files modified

libindicator/indicator-object-marshal.list (+2/-1)
libindicator/indicator-object.c (+23/-2)
libindicator/indicator-object.h (+3/-1)

To merge this branch:

bzr merge lp://qastaging/~3v1n0/libindicator/scroll-event-support

Undecided

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Ted Gould (community)		2011-01-26	Approve on 2011-01-27
Review via email: mp+47608@code.qastaging.launchpad.net

Description of the change

libindicator support the new "scroll-entry" signal (which get called using the target entry as parameter too) I think that maybe we could simply rewrite the "scroll" signal, but I kept it there for compatibility reasons; however, actually just the indicator-sound should be fixed in case of switch...

This is part of the Indicators "scroll-event" support patch, see bug #708180

Revision history for this message

Ted Gould (ted) wrote on 2011-01-27:

69 -#define INDICATOR_OBJECT_SIGNAL_ENTRY_ADDED "entry-added"
70 +#define INDICATOR_OBJECT_SIGNAL_ENTRY_ADDED SCROLL "entry-added"

I'm not sure what the SCROLL here is for. Otherwise it looks good.

review: Needs Information

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2011-01-27:

> 69 -#define INDICATOR_OBJECT_SIGNAL_ENTRY_ADDED "entry-added"
> 70 +#define INDICATOR_OBJECT_SIGNAL_ENTRY_ADDED SCROLL "entry-added"
>
> I'm not sure what the SCROLL here is for. Otherwise it looks good.

Ops, it was a typo (or bad paste :P)... I'm fixing it, sorry.

lp://qastaging/~3v1n0/libindicator/scroll-event-support updated on 2011-01-27

396. By Marco Trevisan (Treviño) on 2011-01-27

Removed typo in code.

Pasted a more SCROLL value :P

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2011-01-27:

Ah, let me know if you'd prefer to replace the old "scroll" signal with the new one, instead of adding one more (so using an extra slot of the struct for the very same reason); the current implementation could be considered not optimized and containing a duplicate.

In this case, indicator-sound (the only depending on this, right now) should be updated too, to use the scroll_entry signal instead of the scroll one.

Revision history for this message

Ted Gould (ted) wrote on 2011-01-27:

On Thu, 2011-01-27 at 11:37 +0000, Treviño (Marco Trevisan) wrote:
> Ah, let me know if you'd prefer to replace the old "scroll" signal with the new one, instead of adding one more (so using an extra slot of the struct for the very same reason); the current implementation could be considered not optimized and containing a duplicate.
>
> In this case, indicator-sound (the only depending on this, right now) should be updated too, to use the scroll_entry signal instead of the scroll one.

Yeah, from a coding perspective it's not a big deal. But every time we
break API we kinda mess with the distro folks in a bad way. So it's
worth not doing that if we don't need to. Next time we break ABI I'll
reclaim the reserved.

Thanks for fixing the typo!

review approve

review: Approve