vkrause added subscribers: dvratil, vkrause.
vkrause added a comment.
Thanks for working on this! Some general thoughts:
- Is KDeclarative is the right place for this? It's a module on the way out
in KF6, it is hard to build for Android due to its dependency chain (which
isn't even needed for this), while at the same time forcing ABI stability on
this basically immediately without much chance for battle testing this.
- This currently represents a flat list of calendars, which matches Android
IIRC, but not Akonadi (which treats calendars as folders and thus
hierarchical). That might not be a problem as long as we treat Akonadi as a
single calendar and don't expose the different backends on this level at all,
but IIUC @dvratil was argueing to move away from that approach.
INLINE COMMENTS
> calendarentry.cpp:26
> +CalendarEntry::CalendarEntry(const QString &id, const QString &description,
> const QString &icon, CalendarType type, KCalendarCore::Calendar::Ptr calendar)
> + : d(new CalendarEntryPrivate)
> +{
this is leaked it seems, maybe make d a unique_ptr?
> calendarentry.h:27
> +class CalendarEntryPrivate;
> +
> +class Q_DECL_EXPORT CalendarEntry : public QObject
add at least very minimal class-level API docs so this shows up on api.kde.org
> calendarentry.h:34
> + Q_PROPERTY(QString icon READ icon CONSTANT)
> + Q_PROPERTY(CalendarType type READ type CONSTANT)
> + Q_PROPERTY(KCalendarCore::Calendar::Ptr calendar READ calendar CONSTANT)
"type" is quite generic, maybe rather something like "permission"?
> davidedmundson wrote in calendarentry.h:57
> I don't understand what this is for.
>
> Is it like Calendar::save ?
synchronize() maybe? ie. avoid abbreviations in API
REVISION DETAIL
https://phabricator.kde.org/D24443
To: nicolasfella, #frameworks, #plasma
Cc: vkrause, dvratil, davidedmundson, dhaumann
