Review Request 118357: Disable the agenda part of the calendar

[Sebastian Kügler]

> On May 30, 2014, 1:27 p.m., Sebastian Kügler wrote:
> > Since the visible flag is now static false, let's comment the agenda item code, so we don't waste memory or CPU cycles. Once done, please ship.
> 
> Martin Klapetek wrote:
>     I specifically opted out of commenting it as that would mean commenting stuff all around (stuff that references the commented out stuff). If really needed, I can do tho. On the other hand, if this is to make things faster, it will make them slow again in the future --> users will observe performance drop ;)
> 
> Sebastian Kügler wrote:
>     That's a pretty weak argument. "Let's keep rubbish in that isn't used, so when we use it, it's as slow as it's now and nobody will notice that we added stuff and slowed it down", essentially. :D
>     
>     The agenda part should be optional in the code, perhaps show / hide dynamically, based on whether the user wants or as agenda items. These are quite complex bits of UI, the lazy-loading helps, but we should still attempt to make it not load stuff that isn't necessary.
>     
>     So let's do it right.

Ah, btw ... commenting stuff all around is not needed: You can just check within the code if references are valid and make it conditional, that's more future-proof as well.


- Sebastian


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


On May 27, 2014, 4 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> -----------------------------------------------------------
> 
> (Updated May 27, 2014, 4 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a proper agenda backend and should just be hidden. Here's a patch simply hiding the left part - it's easier to do just "visible: false" than comment it out and then comment out/remove all the lines referencing parts of the agenda, also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove plasma config as the popup size seems to be saved and so after this patch you may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -----
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Calendar without agenda
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140530/5e051f90/attachment.html>