[Dextrose] [PATCH v2] uy#698: Rectifying "Activity-icons not displaying, if notification-messages are shown"

[Sascha Silbe]

Excerpts from Ajay Garg's message of 2011-12-21 21:08:47 +0100:

> Note that this patch is to be applied in full, and NOT over the
> version-1 patch.
>
> Reverted to the previous-behaviour, wherein the
> notification-message-icon is shown at the end of all activity-icons
> (Thanks Anish and Estaban).
> 
> Note that, simply loading the journal, before setting up the
> top-panel-frame, was not working.
> 
> Thus, "gio.File.monitor_file" has been used as the signalling
> mechanism.  The top-panel-frame-setup (wherein the
> startup-notification-messages may be added) gets processed, ONLY after
> the journal has been set-up.
[...]

I miss a good description:

What is the problem on the UI side?
Answer: The Journal activity icon doesn't appear as active activity icon
on start-up if a notification gets added before the Journal has been
loaded.

What is the problem on the technical level? I.e. why does this happen?
(I don't know and that's the most important problem with this patch;
without this I can't tell whether it will work at all; race conditions
are tricky beasts)

What was done to fix this?
(Delay Frame start-up until after the Journal started; you already
mention this, but not clearly enough)

How was it implemented?
(Using flag file for intra-process communication, sugar-session waiting
for trigger from ActivitiesTray. Again, you mention it, but in a rather
low-level way that's hard to understand. gio.File.monitor_file() could
serve a lot of different purposes. It's not clear that you're using it
as part of a flag file based intra-process communication method).

The summary line (mail subject) should explain what you changed (e.g.
"Delay Frame setup until after the Journal was loaded"), not why (except
for mentioning a ticket number, preferably an Sugar Labs one) or how. 


> ---

The changelog should go below this line.


On the technical level, there are three issues with your patch:

1. It probably doesn't fix the problem, but rather works around it.
   Adding a description of what is actually happening will help
   considering whether this is the case.

2. ActivitiesTray isn't the right place to pull the trigger. It's a
   widget (i.e. UI), not a backend interface. It's possible that the
   current code doesn't properly distinguish these two in the code you
   touched, but if that's the case it should be mentioned in the patch
   description and either fixed by a follow-up patch or explained in a
   new ticket on bugs.sl.o so we can fix it later. Don't pile new hacks
   on old ones, it'll fall apart faster than you can run (resp. code).

3. There are much better *intra*-process communication primitives than
   using flag files.


Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.sugarlabs.org/archive/dextrose/attachments/20111221/f2d2d56c/attachment.pgp>