> 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
