Code review comment for lp:~zsombi/ubuntu-ui-toolkit/alarm-fetch-fix

> // do we get segfault because this is missing?
>
> That's a strange comment. I'd assume you want to a) use deleteLater because
> you know that it can be used until the next main loop iteration, and have the
> comment say that, idealy by what b) or you scratch both the comment and the
> deleterLater if this isn't the case c) or FIXME: Work-around unexplained use
> of fetchRequest

Right, that comment was left there by mistake :)

>
> // with EDS we need to query the occurrences separately as the fetch reports
> only the main events
>
> Is there a bug on fixing the EDS backend?

There was a bug, the fix had landed way before... More than 1 month ago... The fix was made right after the Toolkit's alarm fetch got broken. However, the same logic works well with the memory manager, so basically there's no difference in logic. We skip the occurrences in the main fetch loop anyway, and only memory manager reports occurrences there (in completeFetchAlarms()). If EDS would report occurrences in the same way, we would be in trouble again, performance wise that's unacceptable.

>
> // if we are on Monday, the first alarm occurrence is gonna be on Wednesday!
>
> It's very bad to have different test results based on when it runs. I'd rather
> you mock the date or tweak the environment to test both.

I'll redo the test as it supposed to test whether the first occurrence is the right one. For instance if today is Monday and the occurrence is set to Sunday and Monday, the first occurrence of the alarm should be on Sunday the same week.

>
> QString envManager(qgetenv("ALARM_BACKEND"));
>
> Would be good to have a test case to check that this works. And it might best
> to unconditionally set it so that you don't have to scratch your head on
> whether it was picked up or not but simply fails or works.

This is tricky, as we only have two managers, eds and memory, and eds might not even be available, so I cannot be sure whether that will work. I can only test with arbitrary garbage unavailability....