Tuesday, 2020-06-09

*** zbenjamin_ is now known as zbenjamin01:40
dcalisteHello chriadam and flypig, how are you ?07:02
chriadamhello dcaliste, I'm well thanks!  how are you?07:03
dcalisteFine thank you. What about starting with the bug fixes in caldav ?07:04
chriadamthe only question I had about that MR#68 was why at line 516 do we only load() rather than loadSeries()?07:05
chriadamI guess that one is to load the parent series event?07:05
dcalisteI think I answered it yesterday. It's because we only need one representant of the given UID to be added to mLocalModifications. Then the for loop for modifications will take care of loading the series for the PUT.07:06
flypigSorry I'm a bit late. Good morning dcaliste, chriadam. I'm glad to hear you're recovered dcaliste.07:07
dcalisteThank you flypig.07:08
dcalisteI've added the enbaleService() and add the contributes as you suggested for the nemo-qml-plugin-calendar MR on visibility.07:08
chriadamoh, that makes sense.  somehow I didn't see your reply yesterday07:09
flypigGreat, thank you dcaliste. Then I'll go ahead and merge it (unless you have any objections chriadam on: https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/55 )07:10
chriadamnope, go for it07:10
dcalisteNo problem chriadam, I think I too hastily pushed the resolve thread button...07:10
chriadamcaldav#68 and mkcal#38 LGTM then, I will merge tomorrow unless anyone objects07:10
chriadamsame for NQPC#59 and mkcal#40 although I still need to run those unit tests on device first07:11
dcalisteThank you chriadam for both.07:12
chriadamfor nqpc#59, a comment to the "if" branch where we were discussing would be useful IMO07:13
chriadamfor clarity07:13
dcalisteOk, I'll add it, no problem.07:13
flypigConcerning the enableService() change on MR55, a question for you both, is that needed on line 849 as well? Or is this just unnecessary in general.07:13
flypigNot 849, 840; sorry misread,07:14
dcalisteSorry flypig, do you mean in the for loop ? There is already a enableService() call with a selected service. Or do you mean before the for loop ?07:15
flypigI mean, to clear out the service selection when the function returns.07:15
chriadamah.  `const bool ret = account->enabled();  account->selectService(); return ret;`07:16
flypigInside the for loop, yes.07:16
flypigYes, exactly chriadam.07:16
chriadamyes, I think so.  avoid mutating the state of the account parameter07:16
dcalisteAh, I see, sorry. Thank chriadam. Yes I agree also that there should be one call to enableService() there also.07:17
dcalisteI'm going to add it.07:17
flypigGreat, thank you.07:18
chriadamI assume you meant selectService() ;-)  cheers07:18
flypigYes, you're right, I caused confusion :( Apologies :)07:19
dcalistechriadam, sure my mistake, yes selectService()07:19
chriadamnext ones on our list to discuss would be the UI timezone selection support ones.  I haven't had a chance to look at those recently, but pvuorela is keen to get those in, so I will make time this week to review and test those also.  hopefully blam and pvuorela are able to do so also (not sure if flypig has time alongside the browser work he's doing?)07:20
flypigThat's 252 and 38?07:21
dcalisteIt's https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/5107:22
dcalisteBut for the UI part, you're right it's 252 and 38.07:23
chriadamI'm mostly unprepared to discuss those, unfortunately.  maybe I could ask: did you get any feedback from Pekka, Joona or Martin about the UI design side of things?07:24
dcalisteYes, pvuorela already commented much on the UI part and I think, we agree to use a valueButton that opens a modified time zone picker.07:25
dcalisteThe modifications are used to include UTC or no timezone choices.07:25
chriadamso there are no design roadblocks, only technical implementation review to complete?07:27
dcalisteI think so, and maybe general approving. I've addressed last week the implenetation remarks pvuorela did on the timezone picker.07:27
chriadamexcellent, thank you07:28
pvuorelaheya. was hoping i could have managed to test the latest version, but indeed it seemed good.07:28
dcalisteThank you pvuorela. I'll monitor the MR to see if you have additional remarks when you will have time to test the latest changes.07:29
chriadamregarding the libcommhistory things - I know blam took a look at it, but I didn't get a chance to, yet.  let's get to that one next week, I think07:30
dcalisteOk, no problem.07:30
dcalisteI'm going back to CalDAV, where there is a recent modification that I would like to discuss : https://git.sailfishos.org/mer-core/buteo-sync-plugin-caldav/merge_requests/7007:30
dcalisteIf you have time to.07:30
chriadamof coruse07:31
chriadamlet me check it07:31
chriadamhaving to re-get every resource after upsync leaves a bad taste in my mouth07:33
chriadamit basically doubles the chance that we'll hit network errors / etc07:34
chriadambut I guess it's necessary07:34
chriadamhow do we organise the transactions to ensure that an error in the subsequent "GET" doesn't cause any atomicity issues?07:34
chriadamah, well, no different I guess, since we had to update the ETag value for each event anyway07:36
dcalisteFirst, I don't like it much neither, but the comment from the guy of OpenXchange leaves few possibilities and it clearly make sense in my opinion.07:36
chriadamindeed, I concur07:36
dcalisteBut the way it is handled after the MR is quite simple in my opinion: the fetched events modified on server are pulled later anyway, so I'm just increasing the list of fetch URIs.07:37
dcalisteThen the local update is done as for true upstream modifications.07:37
dcalisteThe etags are already received and updated in conjunction with the iCal data.07:38
dcalisteI think this makes the process more robust in fact, while paying more data transfer sadly...07:39
flypigWhat are the semantics of the SEQUENCE number? Would it make sense to increase the client sequence number pre-emptively, in case the later update fails?07:40
dcalisteIf the network fails before we receive the "updated" events from server, then the etag is not saved anyway, and we'll retrieve the server version at the next sync.07:41
flypigOkay, so it's safe either way.07:41
dcalisteflypig, I don't think so, because this is server dependant. Some server may not change the PUT data anyway and send the etag back immediately. (in that case we don't do a GET)07:42
flypigOkay, thanks; makes sense.07:42
chriadamunless there are issues raised, I will merge that one tomorrow also, along with the other one (will piggyback through CI on the same tag)07:44
chriadamcan you briefly describe the recurrence rule PRs?  I haven't had a chance to check those in any detail, I guess we will need to discuss them more at next meeting, but an overview might help me digest07:45
dcalistechriadam, I'll answer your question in Gitlab after a longer thinking period, but I think we won't have any duplicates in the fetch list, since we either push to the server or get, but cannot ask for both. So the fetch list cannot contains something that we also would like to push.07:45
dcalisteAbout the recurrence rule MR, there are two addition in two commits (there is a third one in fact to modify slightly implementation):07:46
dcaliste- first is to handle the "3rd Monday every month case",07:46
dcaliste- second is for "every week on Monday, Thursday and Sunday" cases.07:47
dcalisteIt's the same on UI, two commits to add the two possibilities.07:47
dcalisteThe underlying implementation in kcalcore already exists, so the idea was to extend the CalendarEvent::Recur enum with the two cases.07:47
dcalisteThe nth label is computed on UI side.07:48
dcalisteFor the day of week, when selected, a switch row with days is appearing after the recurrence value button.07:48
dcalisteWe can continue to discuss this next week. I need to move to the doctor now.07:49
chriadamsounds good, thanks again for your efforts07:49
chriadambest wishes for the recovery of your hand!07:50
dcalisteI've an appointment in ten minutes ;)07:50
chriadamhopefully the appointment brings good news!07:50
dcalisteHave a nice week, thank you.07:50
chriadamyou too!07:50
flypigThanks dcaliste.07:50
chriadamgood night!07:50
*** Redfoxmoon_ is now known as Redfoxmoon08:35
*** frinring_ is now known as frinring09:58
*** Guest4518 is now known as fledermaus12:13
attahHmmm, are 1486 builds supposed to be 1.5MB nowadays?17:21
attahdid it just statically link against something with a contagious license?17:26
malattah: what do you mean?17:45
attahI built one version for arm: 150k, and the mandatory i486 package is 10x that17:46
attahand a lot of it in the binary17:46
attah(apps building with sfdk, that is)17:55
mkolmanattah: check if the i486 version is stripped19:16
mkolmanthat could explain the difference19:16
*** Ischwitch is now known as Ingvix21:22

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