Code review comment for lp:~renatofilho/indicator-datetime/notify-missing-alarm

Revision history for this message
Charles Kerr (charlesk) wrote :

More code-level comments inline. Most are minor/suggestions, two NF

A few larger issues:

1. Again I'd /strongly/ prefer that ical events pass in their source desktop id as an x-prop. Not only would that eliminate the need for libubuntu-app-launch2-dev >= 0.9 but it would also let us have per-app sources, eg alarms wouldn't show up under the calendar icon. The approach in this patch works for calendars but assumes that the whole world is a calendar.

2. If we must use libubuntu-app-launch2-dev, can we confirm that it's going to make it into Xenial? It's awfully early to be breaking build compatibility with Xenial

review: Needs Fixing

« Back to merge proposal