*** zbenjamin_ is now known as zbenjamin | 01:32 | |
chriadam | hello dcaliste, how has your week been? | 07:01 |
---|---|---|
dcaliste | Hello chriadam, it's alright thank you. What about yours ? | 07:02 |
chriadam | yeah, not too bad, thanks :-) | 07:02 |
chriadam | discussed 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 week | 07:03 |
dcaliste | That would be great. Thank you. | 07:04 |
chriadam | the 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 |
chriadam | I 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 |
dcaliste | In the latter, there was the opened question of the property name for the occurrence start time in event timezone. | 07:05 |
dcaliste | Well, the previous latter ;) | 07:05 |
dcaliste | I mean https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/51 | 07:05 |
dcaliste | I called it 'startTimeZone', but it's not easily understandable, could even be misleading, because it's a dateTime, not a zone... | 07:07 |
dcaliste | As I said in the comments, 'startTimeInEventTimeZone' is more accurate but may be too long. | 07:08 |
chriadam | I will add a comment to the PR and poke pvuorela and flypig :-) | 07:08 |
dcaliste | Ok, thanks. | 07:09 |
dcaliste | Besides, pvuorela found a bug in the visibility commit in nemo-qml-plugin-calendar. | 07:09 |
dcaliste | When the old QSetting is excluding a notebook, the current commit does not allow to remove this exclusion... | 07:10 |
dcaliste | I've created a new MR yesterday to fix this : https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/60 | 07:10 |
chriadam | oh, thank you very much | 07:14 |
chriadam | will check that one | 07:14 |
chriadam | seems like flypig also is checking that one, great | 07:14 |
flypig | Just one small comment, without having tested it. | 07:15 |
dcaliste | Thank you also flypig for finding this issue with the google calendar addition. | 07:16 |
flypig | No problem. That issue aside, it worked really nicely. | 07:16 |
flypig | I just need to now test your updated PR. | 07:16 |
dcaliste | chriadam, 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 |
chriadam | agh silly me | 07:17 |
chriadam | will edit comment, thanks | 07:17 |
dcaliste | Or Am I wrong myself ? | 07:18 |
dcaliste | Wondering now, let me check again... | 07:18 |
dcaliste | Ah, don't touch anything, your comment is accurate. | 07:19 |
chriadam | ah, cool | 07:19 |
dcaliste | I misread the diff, and looked at the wrong file... My bad. | 07:19 |
chriadam | all good, I wasn't sure, it's been a while since I reviewed that PR so entirely possible that I'd misremembered | 07:20 |
chriadam | flypig: if you have any comments for the property name etc, would be appreciated! | 07:20 |
chriadam | nqpc#51 | 07:20 |
flypig | Thanks for highlighting it. I didn't consider it at all yet, but will take a look. | 07:22 |
chriadam | I'm beginning to like "startTimeInTz" | 07:22 |
chriadam | but no strong opinion. naming things is hard. | 07:22 |
dcaliste | Yeh, besides the abbreviation of time zone, it's short enough and properly descriptive. | 07:23 |
flypig | Given the context, I think the meaning of Tz is clear. | 07:24 |
dcaliste | Ok, 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 |
dcaliste | s/tupo/typo/ | 07:25 |
chriadam | sounds like a good plan to me, thank you very much | 07:25 |
dcaliste | Talking about calendar UI, I'm proposing a small UI change for multi event time labels, see jolla-calendar!264 | 07:26 |
chriadam | the other PRs I remember needed discussion was kcalcore MR#13 and caldav MR#57, this is related to exclusive dtend | 07:26 |
chriadam | my 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 too | 07:27 |
chriadam | I had a WIP PR for google IIRC | 07:27 |
chriadam | ah, seems I had a PR for sailfish-eas also | 07:30 |
chriadam | who knew :-D | 07:30 |
dcaliste | Yes, that's right, these MRs requires some synchronisation. | 07:30 |
dcaliste | One should look for every places where there is a iCal serialisation. | 07:31 |
dcaliste | I think, I need to add a MR myself in nemo-qml-plugin-calendar for the icalconverter tool. | 07:31 |
chriadam | nqpc#46 should do that already IIRC | 07:31 |
dcaliste | Ah, yes, right, I forgot this one. | 07:33 |
chriadam | I 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 |
chriadam | will check that | 07:34 |
chriadam | aside 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 |
chriadam | and I will rebase mine, and merge some time this week | 07:35 |
dcaliste | Hopefully yes. That would be great. I'll do the rebase soon. | 07:35 |
chriadam | thanks very much. no rush of course. | 07:36 |
chriadam | can be enxt week if you're busy etc | 07:36 |
chriadam | Ok, 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 up | 07:36 |
chriadam | maybe we can discuss that one next week instead - sorry | 07:36 |
dcaliste | Sure, no problem neither. | 07:37 |
chriadam | cool | 07:37 |
chriadam | was there anything I've missed, or anythign else you'd like to discuss this week? | 07:37 |
flypig | The 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 |
chriadam | or flypig anythign from you? | 07:37 |
flypig | No, nothing from me. I'm a bit overwhelmed by the number of PRs you're both juggling. | 07:38 |
flypig | *sync <> serialisation | 07:38 |
chriadam | dcaliste is extremely productive! as always, huge thanks for your hard work and great efforts, dcaliste | 07:39 |
dcaliste | Yeh, me too flypig ;) And we're not yet ready for the KCalCore upstream migration... | 07:39 |
flypig | :) | 07:39 |
flypig | Concerning 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 |
flypig | I don't have any specific suggestions though. | 07:41 |
chriadam | hrm, you mean update the "old" property name to be "startTimeInLocalTz" instead? | 07:41 |
chriadam | rather than just "startTime"? | 07:41 |
chriadam | problem with that is that it involves changing more existing code, IMO | 07:41 |
flypig | Yes, I agree, changing the existing values doesn't sound like a great idea. | 07:42 |
dcaliste | The 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 |
flypig | Okay, then that wouldn't be appropriate. | 07:42 |
flypig | I'll ponder on it further, but I don't think I'll come up with anything better than you already have. | 07:43 |
chriadam | thanks - yup, naming is hard | 07:43 |
dcaliste | Thank you flypig. | 07:43 |
flypig | np | 07:44 |
dcaliste | Just one last word, if you think it's worth to ask Martin opinion on jolla-calendar!264 ? | 07:44 |
chriadam | will poke him now, sec | 07:44 |
dcaliste | It's about multi-day labels to avoid things like 16:00-23:59. | 07:45 |
chriadam | have poked, but no response from Martin or from Joona - they must be in a meeting currently | 07:47 |
chriadam | IIRC discussing ambience stuff | 07:47 |
chriadam | but, hopefully they will read and response via the PR shortly | 07:48 |
dcaliste | No problem, it can be async ;) | 07:48 |
chriadam | great - thanks for that one too! | 07:48 |
dcaliste | Anyway, thank you very much and I think that's it for me. | 07:48 |
chriadam | ok, if nothing else to discuss, I hope that you have a great week and that your hand continues to heal! | 07:48 |
dcaliste | It's verrrry slow... But it's alright. Thank you. Have a nice week too. | 07:49 |
chriadam | gnight! | 07:49 |
*** frinring_ is now known as frinring | 09:16 | |
*** jameshjacks0njr_ is now known as jameshjacks0njr | 14:16 | |
johansmitsnl | Is the Sony XPERIA 10 II phone also supported or just the Sony XPERIA 10? | 17:29 |
mal | website says Xperia 10 and Xperia 10 Plus devices are supported | 17:31 |
johansmitsnl | mal: I noticed, but the normal Sony XPERIA 10 is already getting dificult to buy | 17:34 |
attah | piggz: pushed something that is a reasonable start | 18:35 |
piggz | attah: ok,ill try it | 18:51 |
*** zama_ is now known as zama | 22:39 |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!