Tuesday, 2020-06-16

*** zbenjamin_ is now known as zbenjamin01:32
chriadamhello dcaliste, how has your week been?07:01
dcalisteHello chriadam, it's alright thank you. What about yours ?07:02
chriadamyeah, not too bad, thanks :-)07:02
chriadamdiscussed with pvuorela yesterday about some of your open PRs, he wants to get the recurrence rule changes in, so I will review those also this week07:03
dcalisteThat would be great. Thank you.07:04
chriadamthe timezone PRs I don't remember whether Martin had any more feedback on how the tz selection works, but from my POV I think they're looking good.07:04
chriadamI know that flypig is also following up on the google calendar PR related to the recent nqpc change, hopefully that one will go in either today or tomorrow.07:04
dcalisteIn the latter, there was the opened question of the property name for the occurrence start time in event timezone.07:05
dcalisteWell, the previous latter ;)07:05
dcalisteI mean https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/5107:05
dcalisteI called it 'startTimeZone', but it's not easily understandable, could even be misleading, because it's a dateTime, not a zone...07:07
dcalisteAs I said in the comments, 'startTimeInEventTimeZone' is more accurate but may be too long.07:08
chriadamI will add a comment to the PR and poke pvuorela and flypig :-)07:08
dcalisteOk, thanks.07:09
dcalisteBesides, pvuorela found a bug in the visibility commit in nemo-qml-plugin-calendar.07:09
dcalisteWhen the old QSetting is excluding a notebook, the current commit does not allow to remove this exclusion...07:10
dcalisteI've created a new MR yesterday to fix this : https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/6007:10
chriadamoh, thank you very much07:14
chriadamwill check that one07:14
chriadamseems like flypig also is checking that one, great07:14
flypigJust one small comment, without having tested it.07:15
dcalisteThank you also flypig for finding this issue with the google calendar addition.07:16
flypigNo problem. That issue aside, it worked really nicely.07:16
flypigI just need to now test your updated PR.07:16
dcalistechriadam, thanks for your comment, but startTimeInEventTimeZone is supposed to replace startTimeZone, not startDateTime, which is the date time in local time zone, used by the UI to display the time of the occurrence.07:17
chriadamagh silly me07:17
chriadamwill edit comment, thanks07:17
dcalisteOr Am I wrong myself ?07:18
dcalisteWondering now, let me check again...07:18
dcalisteAh, don't touch anything, your comment is accurate.07:19
chriadamah, cool07:19
dcalisteI misread the diff, and looked at the wrong file... My bad.07:19
chriadamall good, I wasn't sure, it's been a while since I reviewed that PR so entirely possible that I'd misremembered07:20
chriadamflypig: if you have any comments for the property name etc, would be appreciated!07:20
chriadamnqpc#5107:20
flypigThanks for highlighting it. I didn't consider it at all yet, but will take a look.07:22
chriadamI'm beginning to like "startTimeInTz"07:22
chriadambut no strong opinion.  naming things is hard.07:22
dcalisteYeh, besides the abbreviation of time zone, it's short enough and properly descriptive.07:23
flypigGiven the context, I think the meaning of Tz is clear.07:24
dcalisteOk, we can let it settle for some more days, and if nothing better appears in between, I'll update the MR (correct the tupo chriadam you mentioned) and propagate the change in the UI MR also.07:25
dcalistes/tupo/typo/07:25
chriadamsounds like a good plan to me, thank you very much07:25
dcalisteTalking about calendar UI, I'm proposing a small UI change for multi event time labels, see jolla-calendar!26407:26
chriadamthe other PRs I remember needed discussion was kcalcore MR#13 and caldav MR#57, this is related to exclusive dtend07:26
chriadammy question was: what work did I need to do for that one - was sailfish-eas plugin needing update?  google?  pekka mentioned timed I can check that too07:27
chriadamI had a WIP PR for google IIRC07:27
chriadamah, seems I had a PR for sailfish-eas also07:30
chriadamwho knew :-D07:30
dcalisteYes, that's right, these MRs requires some synchronisation.07:30
dcalisteOne should look for every places where there is a iCal serialisation.07:31
dcalisteI think, I need to add a MR myself in nemo-qml-plugin-calendar for the icalconverter tool.07:31
chriadamnqpc#46 should do that already IIRC07:31
dcalisteAh, yes, right, I forgot this one.07:33
chriadamI think that should be all the cases, unless flypig can think of any other case where we do ical serialisation (hmm.  maybe sharing events via bluetooth etc?)07:34
chriadamwill check that07:34
chriadamaside from that, are you happy/confident that we can merge all those now?07:35
chriadam(and if so, any chance you could also rebase your PRs: kcalcore#13 + caldav#57)07:35
chriadamand I will rebase mine, and merge some time this week07:35
dcalisteHopefully yes. That would be great. I'll do the rebase soon.07:35
chriadamthanks very much.  no rush of course.07:36
chriadamcan be enxt week if you're busy etc07:36
chriadamOk, the only other thing to discuss is the commhistory change etc.  unfortunately I still haven't reviewed that properly.  I know blam took a look but I never followed up07:36
chriadammaybe we can discuss that one next week instead - sorry07:36
dcalisteSure, no problem neither.07:37
chriadamcool07:37
chriadamwas there anything I've missed, or anythign else you'd like to discuss this week?07:37
flypigThe only place I recall hitting ical sync was the EAS event emails. From what you say it looks like you have that covered.07:37
chriadamor flypig anythign from you?07:37
flypigNo, nothing from me. I'm a bit overwhelmed by the number of PRs you're both juggling.07:38
flypig*sync <> serialisation07:38
chriadamdcaliste is extremely productive!  as always, huge thanks for your hard work and great efforts, dcaliste07:39
dcalisteYeh, me too flypig ;) And we're not yet ready for the KCalCore upstream migration...07:39
flypig:)07:39
flypigConcerning that variable naming. Maybe it's worth considering 'local' as an alternative to 'Tz'. I'm not sure how it would fit, but just something to think about.07:40
flypigI don't have any specific suggestions though.07:41
chriadamhrm, you mean update the "old" property name to be "startTimeInLocalTz" instead?07:41
chriadamrather than just "startTime"?07:41
chriadamproblem with that is that it involves changing more existing code, IMO07:41
flypigYes, I agree, changing the existing values doesn't sound like a great idea.07:42
dcalisteThe problem with local, is that the time zone of the event may not be the time zone of the device. Besides, indeed I didn't want to touch the existing API.07:42
flypigOkay, then that wouldn't be appropriate.07:42
flypigI'll ponder on it further, but I don't think I'll come up with anything better than you already have.07:43
chriadamthanks - yup, naming is hard07:43
dcalisteThank you flypig.07:43
flypignp07:44
dcalisteJust one last word, if you think it's worth to ask Martin opinion on jolla-calendar!264 ?07:44
chriadamwill poke him now, sec07:44
dcalisteIt's about multi-day labels to avoid things like 16:00-23:59.07:45
chriadamhave poked, but no response from Martin or from Joona - they must be in a meeting currently07:47
chriadamIIRC discussing ambience stuff07:47
chriadambut, hopefully they will read and response via the PR shortly07:48
dcalisteNo problem, it can be async ;)07:48
chriadamgreat - thanks for that one too!07:48
dcalisteAnyway, thank you very much and I think that's it for me.07:48
chriadamok, if nothing else to discuss, I hope that you have a great week and that your hand continues to heal!07:48
dcalisteIt's verrrry slow... But it's alright. Thank you. Have a nice week too.07:49
chriadamgnight!07:49
*** frinring_ is now known as frinring09:16
*** jameshjacks0njr_ is now known as jameshjacks0njr14:16
johansmitsnlIs the Sony XPERIA 10 II phone also supported or just the Sony XPERIA 10?17:29
malwebsite says Xperia 10 and Xperia 10 Plus devices are supported17:31
johansmitsnlmal: I noticed, but the normal Sony XPERIA 10 is already getting dificult to buy17:34
attahpiggz: pushed something that is a reasonable start18:35
piggzattah: ok,ill try it18:51
*** zama_ is now known as zama22:39

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