Tuesday, 2020-04-28

*** zbenjamin is now known as Guest2331201:19
*** zbenjamin_ is now known as zbenjamin01:19
dcalisteHello chriadam, I woke up earlier today. If you're free, we may discuss from now and save you time in the evening ?06:03
chriadamhi dcaliste, sure!06:03
chriadamI hope you had a good week!06:03
dcalisteIt was fine, thank you. I wish it was as good for you also.06:04
chriadamyes, wasn't too bad06:05
dcalisteCompared to last week, there are still the two MR changing the listing of notebooks in the calendar app, based on account services instead of notebook visibility.06:06
dcalisteNow that I write it, I think that I still didn't push a commit on top to silence the warning ahen an event has been removed from the manager and the agenda model tries to access it.06:07
dcalisteAs we discussed the meeting before.06:07
chriadamso those two are nqpc#55 and bsps#5806:09
dcalisteExactly, that's them.06:09
chriadamgreat - I think they both look good, have to give them another test before merging. but I guess if you're intending to make a change there, I'll hold off on that for now06:10
dcalisteYes, I'll try to push an additional commit to silence the warning when inquiring for an event that has been removed already. Will do it today.06:11
chriadamno rush, thank you06:11
chriadamI see that nqpc#51 is mostly ok, but was waiting on comment from pvuorela regarding property naming?06:11
chriadamthat's separate of course06:11
dcalisteYes, that's the current status. The naming is really bad at the moment but I cannot find anything better.06:12
chriadamI will poke Pekka again about that one ;-)06:13
chriadamaside from that, I saw the "monthly recurrence on week-day" PRs (jolla-calendar#257 and nqpc#56)06:13
chriadamI haven't had a chance to review them though06:13
dcalisteIt's in CalendarEventOccurrence, we need to differentiate the date/time given in the local time zone (normal start and end properties) from the date / time in the event time zone, because I'm not sure JS can handle the conversion itself.06:14
dcalisteYes, for the last two MRs, I'm trying to implement some mockups provided by Bongo on TJC.06:14
dcalisteThe MR is introducing the recurrence on day of week in a monthly recurrence.06:15
dcalisteSomething like "second Tuesday every month".06:15
chriadammakes sense, something that has been missing for a while, thanks06:16
dcalisteThe support already exists in KCalCore, I've just added the required bindings in the middleware.06:16
dcalisteIt's done as a new value in the Recur enum.06:16
dcalisteThen, the position and the day are taken from the dtstart value.06:17
chriadamPekka also mentioned that Bea and I should try to progress your "list activity for unsaved contacts" PR - need to discuss with her about that one, as I'm not too familiar with libcommhistory06:17
dcalisteKCalCore allows more flexibility, but it's useless in our case, I guess. We put the constraint on the week day of dtstart and the position in the month from the same dtstart value.06:18
dcalisteThere is only one difference with the mockups in TJC, is that we cannot decide between the fourth week or the last. It's automatically chosen to be the last.06:19
dcalisteThe mockups propose to add two possibilities in that case, the fourth and the last, but it complicates the implementation by adding a second enum value to differentiate both cases.06:20
dcalisteSo I decided to go first to an automatic switch to "last" and not giving the possibility to choose between fourth and last.06:20
chriadamI agree, let's keep it simple initially.  it's already a big improvement feature.06:21
dcalisteYeh, and code addition is minor.06:21
dcalisteAbout the other recurring feature that I would like to add from the mockups is the "selected days".06:21
dcalisteMeaning that you recurs weekly on some selected days, like work days, or Monday, Tuesday, Thursday and Friday, because you have kids on Wednesday.06:22
dcalisteThings like that.06:22
chriadamright, training every tuesday and thursday etc06:22
dcalisteKCalCore proposes the same API as the monthlyByDayOfWeek approch.06:23
dcalisteSo it's easy to follow the same path, but...06:23
dcalistechoosing the days, looks like a bit array. So there are two possibilities:06:23
dcaliste- add a new property in CalendarEvent, storing this bit array,06:23
chriadamby bit array, is it just days in week (e.g. 7 bits) or is it days in month (e.g. 31 bits)?06:24
dcaliste- abuse the Recur Enum by booking 127 values and doing the bit array from there.06:24
dcalisteIt's days in week.06:24
dcalisteso 127 possibilities.06:24
chriadamso effectively, only requires adding an extra unsigned char to the CalendarEvent struct06:25
dcalisteMyself, I would prefer the second solution, but I would like to ask your opinion.06:25
dcalisteYou prefer yourself to add only one value in the Recur enum, and inquire another property for the days ?06:26
chriadamI haven't thought it through properly yet, but I worry that (since the Recur enum presumably already has some values) we'd have to extend the enum, then perform bit-shift manipulation to get from the zero-based bit array values to the extended enum value06:26
chriadambut perhaps I'm misunderstanding06:27
dcalisteNo that's the idea ;) That's why I phrase "abuse". There is no bit shift to do though, just substract the enum value of the RecurWeeklyByDays, and then do bit mask on the result.06:28
dcalisteBut having a new property is fine with me also, and less convoluted.06:28
chriadamat first glance, I think I would prefer a new property.  I guess the problem with that is that the client needs to be aware that querying its value only makes sense if the other property's value is the "selectedDays" enum value..06:29
chriadamprobably the lesser of two evils.  can return 0 in that case anyway06:30
dcalisteOh, that's allright I think, because the values will be used only for a specific entry in the combobox values, as in the mockups. Indeed, the return 0 value will do the job when not used.06:30
dcalisteOk, so I'll go for the new property then. Thanks for the direction.06:31
dcalisteAbout the libcommhistory MR, don't hesitate to ask anything, or Bea too also.06:32
chriadamthank you very much, I expect we'll get to it later this week06:33
dcalisteThe main issue, why it was not working before, is an API issue.06:33
dcalisteThe db inquire is done on phone numbers anyway.06:33
dcalisteBut the API just allow to fetch history from a contact id.06:34
dcalisteMy proposition was to add an API to pass a phone number also.06:34
chriadamperhaps related, I know that Venemo has done extensive work on phone number matching, both in libcommhistory but also e.g. voicecall / contacts / etc, so perhaps there is some capability to add API for "match by phone number" now better06:34
chriadamI agree that adding some information (e.g. phone number) makes sense if there is no contact id associated.  in general, I think it's useful to know which channel was used anyway (one person could have multiple devices etc)06:35
chriadamso, sounds good to me ;-)  but let's see what is raised during the discussion06:35
dcalisteThe only thing "strange" in the MR that makes it long to be accepted (with or without modifications), is that the name of the class implies a contact.06:36
dcalisteSo it's strange to add a function to fetch on a phone number.06:36
dcalisteMy proposition was then to add an intermediate new class that works on phone numbers.06:37
dcalisteThe already existing contact history listing is then just a specialisation of the phone number history listing.06:37
chriadamsounds reasonable also06:37
dcalisteYeh, but my C++ skills are not as good as it could be and I mess up a bit with virtual classes. That's what pvuorela raised.06:38
dcalistes/virtual classes/virtual methods/06:39
dcalisteAnd i'm a bit stuck there.06:39
Venemogood morning.06:39
chriadamah, all good, we'll take a look06:39
chriadamhi Venemo06:39
dcalisteHello Venemo.06:39
Venemosorry I missed this meeting, just saw you just highlighted me :P06:40
chriadamwe started early today06:40
dcalisteThanks chriadam, and once again, don't hesitate to ask, on IRC, any question if you find something strange or not understandable in the MR.06:40
chriadamjust briefly discussing dcaliste's MR in libcommhistory06:40
Venemowhat exactly are you proposing wrt phone number matching, dcaliste ?06:40
chriadamallowing user to see communication events with a phone number which hasn't been saved as a contact06:41
dcalisteIt's not exactly about phone number matching, it's about allowing to fetch history for a given number.06:41
VenemoI see06:41
dcalisteCurrently, contact history listing is done by fetching all comm events for all phone numbers of the contact.06:41
dcalisteI'm proposing to add an API to directly attack the db query from a single phone number.06:42
Venemowell, I didn't change the API around it, only the implementation. so the number matching will be more accurate under the hood, but the mechanism to use it hasn't changed06:42
dcalisteSo we can list the history of an unsaved contact without changing anything in the above layers.06:42
chriadamdid you have anything else to discuss this week?06:43
Venemofor example, previously we (mistakenly) matched any two numbers that differed only in the area code, and now that won't happen anymore06:43
dcalisteYeh, I think the two works are orthogonal. Yours making all of this much more robust.06:43
Venemothat's my thought as well06:44
dcalistechriadam, I think that's all for me that week. I'll add the warning mitigation commit today, and then work during the week on the selected day recurrence.06:44
chriadamthanks very much, and thank you as always for your time and effort06:45
chriadamhave a great week!06:45
dcalisteThank you too06:45
VenemoI can take a look at the MR as well, if you want, chriadam06:46
chriadamVenemo: yes, please!06:47
chriadamI havent' had a chance to look at it properly yet, but was planning to, later this week06:47
Venemookay, I will try06:47
dcalisteThank you Venemo.06:47
Venemolibcommhistory always brings tears to my eyes06:47
*** svartoyg is now known as svartoyg_afk07:08
*** leinir_ is now known as leinir07:17
*** frinring_ is now known as frinring08:19
*** Ischwitch is now known as Ingvix15:33

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