Tuesday, 2020-03-10

*** zbenjamin is now known as Guest1243902:57
*** zbenjamin_ is now known as zbenjamin02:57
dcalisteHello chriadam, how are you ?08:09
chriadamhi dcaliste, I'm well thanks!  how are you?08:10
dcalisteWinter sickness, but it will pass…08:10
chriadamoh no!  I hope you recover swiftly08:10
chriadamI must confess that I haven't had time in the past week to take action on any of the topics we discussed at last meeting, unfortunately08:11
dcalisteAnother virus, not the famous one ;) No problem, nothing is in a hurry.08:11
chriadamI will definitely find time to review some PRs this week08:11
dcalisteAnd most of what we discuss last week was for medium term changes.08:12
chriadamso it might be a short meeting tonight, considering that you've been sick and I've not managed to progress things!08:13
chriadamdid you have any topics to discuss?  I need to poke Jaymzz again about the blog post...08:13
dcalisteI think, you notice, but there are these two new MRs:08:13
Venemohey guys08:14
dcaliste- https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/5308:14
dcaliste- and https://bitbucket.org/jolla/ui-jolla-calendar/pull-requests/25508:14
dcalisteHello Venemo. I hope you're fine.08:15
chriadamhi Venemo08:15
Venemototally find, thanks08:15
chriadamI will check08:15
Venemototally fine, thanks08:15
VenemoI hope you are well too08:15
dcalisteIt's about this TJC question : https://together.jolla.com/question/90867/hide-unselected-calendars/08:15
dcalisteThe fact that when creating a new event, non-displayed calendars are proposed in the list, which can be misleading.08:16
chriadamseems indeed good to fix!  thank you.08:16
chriadamI hope pvuorela will have time to check the jolla-calendar one08:16
chriadamthe n-q-p-c side seems simple enough (just exposing the excluded role)08:17
dcalisteYes, I'm using the visible property to hide the excluded calendars, whic is not optimal, but which is minimal changes.08:17
chriadamseems fine to me, but my brain isn't really working right now (context switched from another project recently, so brain not fully working yet ;-)08:19
dcalisteNo problem, besides, I'm going to add MR in buteo-qml-plugin-caldav to discover the read-only property of listed calendars using this https://stackoverflow.com/questions/57116267/determine-access-rights-for-caldav-calendars08:21
dcalisteIt may create cobflict with your caldav MR, but I'll rebase if necessary, or build on top of yours if accepted soon enough.08:22
chriadamah!  that would be very helpful, thanks08:22
dcalisteIt may solve various TJC issue of spurious modification that cannot be uploaded nad make the whole thing not syncable anymore, see https://together.jolla.com/question/223428/sync-error-on-read-only-calendar/08:23
chriadamI'm hoping my MR will go in tomorrow or thursday, gotta fix one more thing, I guess, then it should be fine.08:23
chriadamyep, I have seen such issues in the past08:23
chriadambut wasn't sure how to approach08:23
chriadamthe problem08:23
chriadamwe tried to do something in google calendar (i.e. support owner and write semantics, and also partially allow read-only calendars) but that confused users sometimes, since we didn't expose the read-onlyness to the UI08:24
dcalisteAfter Googling a bit yesterday, I discover there is an RFC for this : https://tools.ietf.org/html/rfc539708:24
chriadamso they would make changes in an event, and expect it to get upsynced, but we'd drop the update silently if it was for a read only calendar08:24
chriadamI know that mkcal notebook has a readOnly flag, but cannot remember if that worked or not08:25
dcalisteWell, read-only in UI makes event not modifiable ???08:25
chriadamthere was some issue, anyway08:25
dcalisteWhen you long press on them, it says "not modifiable"08:25
chriadamah, maybe I'm misremembering08:25
dcalisteAnd the mkcal flag is working, I'm using it for webcal calendars.08:25
dcalisteSo, the only missing bit is the discovery I guess.08:26
chriadamah great08:26
dcalisteNow that we have propfind in caldav plugin, the road is clear I thnk to have it.08:26
chriadamsounds good :-)08:26
chriadamthanks very much08:26
dcalisteAbout the attendee list in qml plugin, what do you think ?08:27
dcalisteShould I give a look to major changes on introducing a minimal struct specifically for agenda models ?08:27
dcalistepvuorela seems to be concern with performence penalties with the full event struct being populated for such models.08:28
chriadamI saw the comment, haven't yet had time to discuss with him08:28
chriadamthe code from what I can see already enumerates the attendees and walks over them08:28
chriadamso what we would save is simply caching the data in the EventData struct member (some memory cost to that)08:28
dcalisteOk, if you think it requires a medium term rework, don't hesitate to ask.08:29
chriadamand some cost to marshalling/demarshalling when sending over the thread boundary in the signal or whatever08:29
chriadamthank you - let's see where the discussion goes, because currently without seeing some hard data, I'd be inclined to keep the PR solution (simple, race-free)08:29
chriadambut of course in some cases memory pressure is very real, so I will try to benchmark some things and see where we get08:30
dcalisteYeh, I more inclined to keep it as simple as possible, but it requires some hard figures to decide. Sorry for making the solving process a bit longer !08:31
chriadamno, thank you for your input, greatly appreciated as always08:32
dcalisteThis may be also the starting point of a reflexion about signaling from mkcal and see what should be designed (and possible) to improve this storageModified() signal, introduce more fine grained ones…08:33
chriadamvery good point...08:33
chriadamamccarthy: oh, hi! long time no chat!  How's things?08:33
chriadamI'll have to poke daniel about organising a dead trolls soon, so we can all catch up!08:34
chriadamdcaliste: I wonder what other observers of that signal we have08:34
chriadamdcaliste: technically, I guess it should be used in contactsd to trigger the "sync on change" automatically08:34
dcalisteAbout this signaling, no specific things in mind at the moment though, just keep this in the back of mind to mature.08:35
chriadambut I don't think I used that one, at the time (the solution we used was fairly hackish unfortunately, my fault)08:35
dcalisteOne other non-related thing : I was wrong about QDateTime not handling ClockTime spec. LocalTime is made for this.08:36
dcalisteI was misleaded by the fact that timeZone() is returning the current one.08:36
dcalisteBut when changing zone, this function is returning the new zone.08:37
dcalisteAnd time doesnot change.08:37
dcalisteSo when switching to upstream kcalcore, we can use this LocalTime. Which bringed me to the infamous rawExpandedEvent() function…08:38
chriadamI don't understand... are you saying: QDateTime clockTime = QDateTime(someDate, someTime, LocalTime);  clockTime.setTimeZone(helsinki);  clockTime.date() == someDate && clockTime.time() == someTime stil?08:38
dcalisteNot exactly.08:38
dcalisteIf you set a time zone, it will change spec to TimeZone spec.08:39
dcalistebut clockTime.timeZone() is return systemTimeZone().08:39
dcalisteWhich made me think at the time, that it was equivalent to KDateTime::LocalZone.08:39
dcalisteBut no, TZ=bar clockTime = someTime and TZ=foo clockTime = sameSomeTime.08:40
chriadamok, so it's just that if you check timeSpec, and that's LocalTime, then you can (for all intents and purposes) ignore the timeZone()08:41
chriadamor am I still misunderstanding?08:41
chriadamcool.  ok, that's good news08:41
dcalisteWhich bringed me to rawExpandedEvents() and its timeSpec (or timeZone) argument…08:41
dcalisteIf we want to keep this function as simple as possible, we should remove this argument and assume it is used to always expand in current time zone.08:42
dcalisteWhich I guess is how it is called now from nemo-qml-plugin-calendar.08:42
chriadamyes.  I wonder what use cases there are for the "specific timezone expansion" parameter in the first place?08:43
chriadamaside from the unit tests ;-)08:43
chriadamwhich.  hrm.  is not a terrible reason...08:43
dcalisteAssuming this, we can avoid the complex move of every LocalTime events to the other time zone without changing time as we're doing now.08:43
dcalisteSince QDateTime LocalTime is having a proper zone, all comparison are working for free, assumling that we're dealing with current time zone.08:44
dcalisteAnd yes, I think the only us case I can forsee is for tests in fact :/08:44
dcalisteFor not using local zone I mean as a timeSpec argument.08:45
dcalisteThe only use case would be to display the calendar in a different time zone than the current.08:45
chriadamI assume the test could simply instead use QLocale or so to set tz to specific tz, perform the test and check expected results for that tz, instead of doing that parametrisation08:45
dcalisteWhich is not supported in the UI, and not usefull in my opinion.08:45
dcalisteIndeed, that's correct. The problem with the test is that we want to test that this function is working in different scenarios,08:46
chriadampvuorela: is that some feature which we might want to support?  e.g. "switch calendar to tz" option, in case you want to see what your schedule will be on a business trip or something?08:46
chriadampersonally, I think "probably not"08:46
chriadamand the resilience we'd gain from simplifying the code has great value, IMO08:47
dcalisteMe neither, and it simplifies a lot rawExpandedEvents().08:47
dcalisteBut well, it's for later anyway, because we need QDateTime for this to work.08:47
dcalisteWhat was broken with KDateTime is that ClockTime has no associated time zone and thus comparisons fails depending on scenarios.08:48
dcalisteUnderstood this this week, so just wanted to let you know about this, in case you need some plannig in advance.08:49
chriadammy brain has turned off unfortunately.  this is in the kcalcore upgrade path isn't it?  i.e. that's when we get rid of kdatetime and switch to qdatetime?08:51
dcalisteI was trying to rebase mkcal upgrade MR and found this in the process.08:52
chriadamright.  in any case, I think that simplifying the rawExpandedEvents() makes sense, in order to avoid parametrisation.  updating the unit tests to verify behaviour properly will be an important step there I guess.08:52
dcalisteYes, what I don't like is that it requires to change code and tests at the same time, when switching to QDateTime…08:53
chriadamaside from that, perhaps I'm not following (in regards to the KDateTime comparisons failing - does that affect us now, currently?  or is that something we handled via the manually settings things to ClockTime etc without the conversion through local-time?)08:53
chriadamtrue, but so long as we're confident that the changes+tests are valid, then I'm fine with that.  after all, we've added a bunch of tests and fixed issues already, so to me there's not too much difference ;-)08:54
chriadambut I agree that the more moving parts, the more chance for accidental regression.  but, sometimes such risks are necessary to move to a better place.08:54
dcalisteAll the patch you did last year in rawExpandedEvents() was to handle these failing comparisons micely with KDateTime. Now, it's fine, but complicated.08:54
dcalistelol s/micely/nicely/08:55
chriadamright, I was hoping there weren't some we missed back then!08:56
chriadamand, yes, complicated.  will be good to simplify, for sure.08:56
chriadamthanks again for looking into this, and pushing the kcalcore upgrade forward08:56
dcalisteStill need to work on the rebase though with my new understanding of the LocalTime spec, because at the moment, it's a mess in mkcal MR ;)08:57
dcalisteSo, let's work on opened MR and I'm going to add the access right discovery for CalDAV.08:58
chriadamsounds good, thank you08:58
chriadamI will definitely check the nqpc/jolla-calendar MRs, and follow up on the PRs we discussed last week aslo08:59
chriadamthanks again for your time and effort, and I hope that you have a good week (and feel better soon!)08:59
dcalisteThanks, got medicine now, it should improve more quickly ;)09:00
dcalisteHave a nice week chriadam. See you.09:00
chriadamthanks!  gnight!09:00
*** ecloud is now known as ecloud_wfh10:01
*** frinring_ is now known as frinring13:15
*** vilpan is now known as Guest1467517:11

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