<table><tr><td style="">nicolasfella added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D24443">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D24443#inline-138439">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dvratil</span> wrote in <span style="color: #4b4d51; font-weight: bold;">calendarentry.h:57</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">I would move <tt style="background: #ebebeb; font-size: 13px;">sync()</tt> from here to be a virtual method on the plugin - <tt style="background: #ebebeb; font-size: 13px;">sync(const CalendarEntry::Ptr &)</tt>. The implementations would reimplement it to handle sync, which feels cleaner than having to connect to <tt style="background: #ebebeb; font-size: 13px;">syncRequested()</tt> signal on each calendar that the plugin owns, and it decouples data (calendar) from logic (plugin).</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I decided to make it a virtual member of CalendarEntry because that simplifies the implementation in calindori. This way it's enough to keep track of the entries, otherwise I'd need to keep track of the plugins too. Also this allows for more fine-grained sync, interesting for large numbers of calendars that are expensive to sync</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D24443#inline-138436">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dvratil</span> wrote in <span style="color: #4b4d51; font-weight: bold;">calendarplugin.h:39</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Unused?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">My idea was to be able to add a d-ptr later if needed without breaking ABI</p></div></div></div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D24443">https://phabricator.kde.org/D24443</a></div></div><br /><div><strong>To: </strong>nicolasfella, Frameworks, Plasma, KDE PIM<br /><strong>Cc: </strong>dkardarakos, vkrause, dvratil, davidedmundson, dhaumann<br /></div>