Tuesday, 2022-03-01

dcalisteGood morning pvuorela. I hope you're well.08:03
pvuoreladcaliste: good morning08:04
dcalisteThank you for your remarks yesterday. I've addressed the points in the mKCal PR and I'm rebasing the factorisation PR in nemo-qml-plugin-calendar.08:05
pvuoreladcaliste: thanks. having this meeting forced me a bit to handle those :) didn't have time to check everything, though.08:07
pvuorelabut not much further comments from my side now.08:10
pvuorelaany preferences on the order of getting current things in?08:10
dcalisteIndeed, these PRs represent a bit of large restructuring. No specific preference on the order to discuss them.08:11
dcalisteI've just pushed the nemo-qml-plugin-calendar PR #25 about factorisation.08:14
dcalisteI've made the CalendarEvent a base class for CalendarEventModification and CalendarImportEvent.08:14
dcalisteI've created a new class also that is CalendarStoredEvent that handle events that are managed by a manager.08:15
dcalisteThe main idea is to share code a bit between these classes that handle events in general.08:16
pvuorelathe amount of deleted lines is clearly more than inserted which is a good sign :)08:16
dcalisteI let you comment in the PR in the coming days, if you have any concerns or remarks.08:19
dcalisteThe PR#26 in nemo-qml-plugin-calendar is a proposition to simplify the creation of exception.08:20
pvuorelasure, i'll do that.08:20
dcalisteLooking at the code, the blocking part that is done in the worker thread to avoid UI freeze is actually the save() call.08:21
dcalisteIf we dissociate the exception in a separate call, this can be done in sync. And later on call save() as for normal events.08:21
dcalisteThis is also respecting the idea of KCalendarCore `createException()` call that is made to be done before modifying the content, while at the moment, we duplicate the event data, modify them and then create an exception and copy there the event data.08:23
dcalisteThis is simplifying a lot also the code to get the recurrenceId, because we don't need to wait for it in an async way.08:23
dcalisteWe can have it immediately in sync, just after the dissociation.08:24
dcalisteThis is an API break though, and there is a companion PR in jolla-calendar, see #314.08:24
dcalisteBut all in all, it's also simplifying the code.08:25
pvuorelayea, not the biggest api adjustment pr on the calendar.08:26
dcalisteAt the moment #26 (dissociation PR) is based on top of #25 (factorisation PR). I'll rebase it once we agree on #25. Even if it's not a dependency. #26 is independant from #25, but it is touching common parts...08:28
pvuorelanod, i'll check the 25 first.08:29
dcalisteThank you.08:29
dcalisteThen you have the PR #27 in nemo-qml-plugin-calendar.08:30
dcalisteIt's mainly to add a regression test on attendees.08:30
dcalisteChecking that sendInvitation(), updateInvitation() are properly called. And that attendee lists are properly updated depending on attendee participation in a modification...08:31
dcalisteIt's based on mKCal#15 to be able to use a specific plugin for the test.08:32
dcalisteOne that won't send emails, but check that parameters to the sendInvitation() and updateInvitation() are properly passed.08:32
pvuorelalooks relatively straightforward.08:33
dcalisteOnce all these PRs are in, I'll publish the one moving from CalendarData::Event to KCalendarCore::Incidence::Ptr as a storing struture in the QML bindings.08:36
dcalisteBesides avoiding duplication in code between the two structures, it would also bring for free support for todos or elements like attachments without updating the QML binding storing structure.08:37
dcalisteHopefully the regression tests are covering enough of the code base ;)08:38
dcalisteTechnically it should also decrease a bit the memory consumption if we don't need to duplicate the data between the worker thread and the manager.08:39
dcalisteI think we won't need for various reasons, but in case we're getting race issues there, we could still copy data instead of using QSharedPointers.08:39
dcalisteDoing so will also solve the attendee issue (at the moment, attendees are fetched async and subject to race conditions), because attendee data will be available to the manager at the same time than the rest of the event data.08:41
dcalisteI wanted also to mention that I'm investigating at the moment on mKCal API additions to be able to differentiate external modifications that requires to reset everything, and modifications that are just saving to DB the modification / addition / deletion of the associated calendar.08:45
dcalisteIf this is conclusive, the main application would be to avoid full reset on every change in the QML bindings.08:46
pvuorelai remember the change signaling wasn't having good info on that now.08:46
dcalisteLike that adding a new event or modifying an existing one could be instantaneous in the UI. Only initial load and external sync would trigger a full reset and the associated 1-2 s of delay.08:47
dcalisteIndeed, I'm adding new signals. I'm creating a transactionId column in a metadata table that is increased on each save.08:48
dcalisteThen, on the fileChanged() callback, the transactionId is checked with the one coming from the last save call.08:48
dcalisteIf they are different, this is an external modification and we need a full reset like before (emitting storageModified),08:49
dcalistebut if they are identical, we just emit a new storageUpdated() signal providing lists of changed incidences.08:50
pvuorelasounds good08:50
dcalisteThen it's up to the user code (here the QML bindings) to deal with internal modification (here, reemit dataLoaded with properly preparated data from the signal lists).08:51
dcalisteBecause, when we save() in the QML binding, the mCalendar in the thread actually contains already the DB content, there is no need to reload the DB actually.08:51
dcalisteOnly when there is an external modification.08:51
dcalisteWe should gain in UI responsivity. But it's still WIP.08:52
dcalisteI'm happy with the mKCal modifications at the moment, but I need to deal with the QML parts now before publishing the PRs.08:52
pvuorelaquite a bunch of things coming up then :)08:53
dcalisteIndeed, nothing directly visible at the UI level, but hopefully some useful restructurings.08:55
dcalisteThank you pvuorela for the meeting. I think I'm done here. Maybe just a last question : do you have any news if your previous proposition is still holding ? Just wondering ;009:01
pvuoreladcaliste: the email? if that got stalling i'll need to check.09:02
dcalisteYes, then thanks !09:03
dcalisteAh, a side note, KCalendarCore found an issue with local time and dtEnd, see https://invent.kde.org/frameworks/kcalendarcore/-/merge_requests/9209:06
dcalisteSo I'm going to update KCalendarCore, but I mentioned the problem of QDateTime()::operator!= and they are preparing a new PR, see https://invent.kde.org/frameworks/kcalendarcore/-/merge_requests/9309:07
dcalisteSo I'm waiting for this MR#93 to land before doing the update in SailfishOS.09:08
attahpiggz: reproduced with 2 of my testing printers, the Canon behaves like yours and the Lexmark just gives up without any action18:44
attahOf course my OKI just works :P18:44
piggzattah: with a JFIF file?18:44
attahNow on to making my own EXIF-header adding tool... codename jfaff18:44
piggzattah: i guess its a corner case, im sure no phone makes jfif files18:49
attahTrue, but plenty of image editing tools it seems18:49
piggzah, cool, i found a good bug for you :)18:49
attahYeah, neat find18:51
attahI must be mad, but this is kinda fun18:51
attahWhat is worse... Qt seems to produce non-EXIF JPEG images too, so if someone is really unlucky with format compatibility they would have their non-jpeg images printed after such a conversion18:54
piggzrinigus: quite amusing ... getting next old app ready for chum .... the build system was so hacked up, at various points in its life, it supported maemo5, harmattan, blackberry10, sailfish and android!19:22
riniguspiggz: time to support plasma mobile? I'll continue with gui release tomorrow, by the looks of it. will be afk for a bit19:35

Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!