Code review comment for lp:~victor-jon/indicator-datetime/drop-phone-code

Revision history for this message
Victor Jonathon (victor-jon) wrote :

1.

> /* Appointment has alarm ? */
  >>Is this necessary ?

Yes, we add a alarm icon if a appointment has alarm.

> /* Get source color */
  >>We don't use source color anymore.

We don't. But we have a open bug about this. It comes down to alarm-icon vs color-code-block. icon was default in rev 297.

>text.value = "";
  >>Wouldn't null check be better ? With new > libecal-1.2 we have to use null to prevent leak.

If we set it to null then we also need to do this

if (appt->summary)
  g_free (appt->summary);

2. Ok, I will.

3. You said that you may implement notification in the datetime itself after libecal port. So I kept it thinking it may become useful. But I can remove it.

« Back to merge proposal