Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- (Updated Nov. 16, 2015, 4:55 p.m.) Status -- This change has been

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review88030 --- Ship it! Ship It! - David Edmundson On Nov. 2, 2015, 5:35

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- (Updated Nov. 2, 2015, 6:35 p.m.) Review request for KDE Frameworks,

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, > > line 202 > > > > > > rather than this much docs, would it make

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, > > line 180 > > > > > > I'd add a dpointer here, even though you

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 31, 2015, 1:08 a.m., David Edmundson wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, > > line 173 > > > > > > Unless needed otherwise, I'd put the UID in

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87768 --- src/declarativeimports/calendar/calendarplugin.cpp (line 36)

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote: > > src/declarativeimports/calendar/daysmodel.h, line 44 > > > > > > I think we need some sort of > > > > QStringList availablePlugins() > > > >

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 28, 2015, 10:15 a.m., Marco Martin wrote: > > not much a fan of adding a public library there, but i guess there aren't > > many other ways. Could it be put into plasma-workspace instead? > > Daniel Vrátil wrote: > Please don't, unless you want to have Akonadi dependency in >

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 28, 2015, 10:15 a.m., Marco Martin wrote: > > not much a fan of adding a public library there, but i guess there aren't > > many other ways. Could it be put into plasma-workspace instead? > > Daniel Vrátil wrote: > Please don't, unless you want to have Akonadi dependency in >

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 28, 2015, 10:15 a.m., Marco Martin wrote: > > not much a fan of adding a public library there, but i guess there aren't > > many other ways. Could it be put into plasma-workspace instead? > > Daniel Vrátil wrote: > Please don't, unless you want to have Akonadi dependency in >

Re: Review Request 125817: Add plugin system for Calendar events

> On Ott. 28, 2015, 10:15 a.m., Marco Martin wrote: > > not much a fan of adding a public library there, but i guess there aren't > > many other ways. Could it be put into plasma-workspace instead? > > Daniel Vrátil wrote: > Please don't, unless you want to have Akonadi dependency in >

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87606 --- Looks good to me now, thanks Martin!!

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 28, 2015, 9:15 a.m., Marco Martin wrote: > > not much a fan of adding a public library there, but i guess there aren't > > many other ways. Could it be put into plasma-workspace instead? > > Daniel Vrátil wrote: > Please don't, unless you want to have Akonadi dependency in >

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- (Updated Oct. 28, 2015, 6:18 p.m.) Review request for Plasma and Daniel

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87563 ---

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87564 --- not much a fan of adding a public library there, but i guess

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 28, 2015, 10:15 a.m., Marco Martin wrote: > > not much a fan of adding a public library there, but i guess there aren't > > many other ways. Could it be put into plasma-workspace instead? Please don't, unless you want to have Akonadi dependency in plasma-workspace. This way we in

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 28, 2015, 9:15 a.m., Marco Martin wrote: > > not much a fan of adding a public library there, but i guess there aren't > > many other ways. Could it be put into plasma-workspace instead? > > Daniel Vrátil wrote: > Please don't, unless you want to have Akonadi dependency in >

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87492 --- One more question/idea that just came to my mind: what about

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, > > line 40 > > > > > > This means that all-day events that span over

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87500 ---

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87512 --- src/declarativeimports/calendar/daysmodel.cpp (line 62)

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, > > line 31 > > > > > > IMO the members here should be hidden in PIMPL.

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, > > line 31 > > > > > > IMO the members here should be hidden in PIMPL.

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- (Updated Oct. 27, 2015, 7:30 p.m.) Review request for KDE Frameworks,

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- (Updated Oct. 27, 2015, 9:53 p.m.) Review request for Plasma and Daniel

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 27, 2015, 10:22 p.m., Daniel Vrátil wrote: > > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h, > > line 31 > > > > > > You are missing implementation for this entire

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87549 --- src/declarativeimports/calendar/daysmodel.cpp (line 67)

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87556 --- src/declarativeimports/calendar/daysmodel.cpp (line 175)

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- (Updated Oct. 27, 2015, 9:10 p.m.) Status -- This change has been

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87550 --- src/declarativeimports/calendar/daysmodel.cpp (line 156)

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 27, 2015, 10:20 p.m., Kai Uwe Broulik wrote: > > What did you do submitting this to the 5.4 branch? :O > On Oct. 27, 2015, 10:20 p.m., Kai Uwe Broulik wrote: > > src/declarativeimports/calendar/daysmodel.cpp, line 184 > >

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 27, 2015, 11:01 p.m., Daniel Vrátil wrote: > > src/declarativeimports/calendar/daysmodel.cpp, line 185 > > > > > > Does it maybe make sense to iterate over the entire set and remove > > *all* events

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 27, 2015, 11:01 p.m., Daniel Vrátil wrote: > > src/declarativeimports/calendar/daysmodel.cpp, line 185 > > > > > > Does it maybe make sense to iterate over the entire set and remove > > *all* events

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87551 --- Aw, sorry, I put in the wrong REVIEW number when committing

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 27, 2015, 9:44 a.m., Daniel Vrátil wrote: > > One more question/idea that just came to my mind: what about some > > configuration interface? For instance in PIM plugin we would definitely > > want to allow users to disable the plugin completely (some people just > > don't like

Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- Review request for Plasma and Daniel Vrátil. Repository:

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/ --- (Updated Oct. 26, 2015, 9:22 p.m.) Review request for Plasma and Daniel

Re: Review Request 125817: Add plugin system for Calendar events

--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125817/#review87470 --- src/declarativeimports/calendar/daysmodel.cpp (line 102)

Re: Review Request 125817: Add plugin system for Calendar events

> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote: > > src/declarativeimports/calendar/daysmodel.cpp, line 150 > > > > > > Is this iteration necessary just to print debug output? > > > > Once we have