| *** zbenjamin_ is now known as zbenjamin | 01:40 | |
| dcaliste | Hello chriadam and flypig, how are you ? | 07:02 |
|---|---|---|
| chriadam | hello dcaliste, I'm well thanks! how are you? | 07:03 |
| dcaliste | Fine thank you. What about starting with the bug fixes in caldav ? | 07:04 |
| chriadam | sure | 07:04 |
| chriadam | the only question I had about that MR#68 was why at line 516 do we only load() rather than loadSeries()? | 07:05 |
| chriadam | I guess that one is to load the parent series event? | 07:05 |
| dcaliste | I 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 |
| flypig | Sorry I'm a bit late. Good morning dcaliste, chriadam. I'm glad to hear you're recovered dcaliste. | 07:07 |
| dcaliste | Thank you flypig. | 07:08 |
| dcaliste | I've added the enbaleService() and add the contributes as you suggested for the nemo-qml-plugin-calendar MR on visibility. | 07:08 |
| chriadam | oh, that makes sense. somehow I didn't see your reply yesterday | 07:09 |
| flypig | Great, 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 |
| chriadam | nope, go for it | 07:10 |
| dcaliste | No problem chriadam, I think I too hastily pushed the resolve thread button... | 07:10 |
| chriadam | caldav#68 and mkcal#38 LGTM then, I will merge tomorrow unless anyone objects | 07:10 |
| chriadam | same for NQPC#59 and mkcal#40 although I still need to run those unit tests on device first | 07:11 |
| dcaliste | Thank you chriadam for both. | 07:12 |
| chriadam | for nqpc#59, a comment to the "if" branch where we were discussing would be useful IMO | 07:13 |
| chriadam | for clarity | 07:13 |
| dcaliste | Ok, I'll add it, no problem. | 07:13 |
| chriadam | thanks! | 07:13 |
| flypig | Concerning 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 |
| flypig | Not 849, 840; sorry misread, | 07:14 |
| dcaliste | Sorry 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 |
| flypig | I mean, to clear out the service selection when the function returns. | 07:15 |
| chriadam | ah. `const bool ret = account->enabled(); account->selectService(); return ret;` | 07:16 |
| flypig | Inside the for loop, yes. | 07:16 |
| flypig | Yes, exactly chriadam. | 07:16 |
| chriadam | yes, I think so. avoid mutating the state of the account parameter | 07:16 |
| dcaliste | Ah, I see, sorry. Thank chriadam. Yes I agree also that there should be one call to enableService() there also. | 07:17 |
| dcaliste | I'm going to add it. | 07:17 |
| flypig | Great, thank you. | 07:18 |
| chriadam | I assume you meant selectService() ;-) cheers | 07:18 |
| flypig | Yes, you're right, I caused confusion :( Apologies :) | 07:19 |
| dcaliste | chriadam, sure my mistake, yes selectService() | 07:19 |
| chriadam | next 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 |
| flypig | That's 252 and 38? | 07:21 |
| dcaliste | It's https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/51 | 07:22 |
| dcaliste | also. | 07:22 |
| flypig | Thanks. | 07:22 |
| dcaliste | But for the UI part, you're right it's 252 and 38. | 07:23 |
| chriadam | I'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 |
| dcaliste | Yes, 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 |
| dcaliste | The modifications are used to include UTC or no timezone choices. | 07:25 |
| chriadam | cool | 07:26 |
| chriadam | so there are no design roadblocks, only technical implementation review to complete? | 07:27 |
| dcaliste | I think so, and maybe general approving. I've addressed last week the implenetation remarks pvuorela did on the timezone picker. | 07:27 |
| chriadam | excellent, thank you | 07:28 |
| pvuorela | heya. was hoping i could have managed to test the latest version, but indeed it seemed good. | 07:28 |
| dcaliste | Thank 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 |
| chriadam | regarding 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 think | 07:30 |
| dcaliste | Ok, no problem. | 07:30 |
| dcaliste | I'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/70 | 07:30 |
| dcaliste | If you have time to. | 07:30 |
| chriadam | of coruse | 07:31 |
| chriadam | let me check it | 07:31 |
| chriadam | urgh | 07:33 |
| chriadam | having to re-get every resource after upsync leaves a bad taste in my mouth | 07:33 |
| chriadam | it basically doubles the chance that we'll hit network errors / etc | 07:34 |
| chriadam | but I guess it's necessary | 07:34 |
| chriadam | how do we organise the transactions to ensure that an error in the subsequent "GET" doesn't cause any atomicity issues? | 07:34 |
| chriadam | ah, well, no different I guess, since we had to update the ETag value for each event anyway | 07:36 |
| dcaliste | First, 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 |
| chriadam | indeed, I concur | 07:36 |
| dcaliste | But 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 |
| dcaliste | Then the local update is done as for true upstream modifications. | 07:37 |
| dcaliste | The etags are already received and updated in conjunction with the iCal data. | 07:38 |
| dcaliste | I think this makes the process more robust in fact, while paying more data transfer sadly... | 07:39 |
| flypig | What 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 |
| dcaliste | If 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 |
| flypig | Okay, so it's safe either way. | 07:41 |
| dcaliste | flypig, 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 |
| flypig | Okay, thanks; makes sense. | 07:42 |
| chriadam | unless 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 |
| chriadam | can 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 digest | 07:45 |
| dcaliste | chriadam, 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 |
| dcaliste | About 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 |
| dcaliste | It's the same on UI, two commits to add the two possibilities. | 07:47 |
| dcaliste | The underlying implementation in kcalcore already exists, so the idea was to extend the CalendarEvent::Recur enum with the two cases. | 07:47 |
| dcaliste | The nth label is computed on UI side. | 07:48 |
| dcaliste | For the day of week, when selected, a switch row with days is appearing after the recurrence value button. | 07:48 |
| dcaliste | We can continue to discuss this next week. I need to move to the doctor now. | 07:49 |
| chriadam | sounds good, thanks again for your efforts | 07:49 |
| chriadam | best wishes for the recovery of your hand! | 07:50 |
| dcaliste | I've an appointment in ten minutes ;) | 07:50 |
| chriadam | hopefully the appointment brings good news! | 07:50 |
| dcaliste | Have a nice week, thank you. | 07:50 |
| chriadam | you too! | 07:50 |
| flypig | Thanks dcaliste. | 07:50 |
| chriadam | good night! | 07:50 |
| *** Redfoxmoon_ is now known as Redfoxmoon | 08:35 | |
| *** frinring_ is now known as frinring | 09:58 | |
| *** Guest4518 is now known as fledermaus | 12:13 | |
| attah | Hmmm, are 1486 builds supposed to be 1.5MB nowadays? | 17:21 |
| attah | i486... | 17:24 |
| attah | did it just statically link against something with a contagious license? | 17:26 |
| mal | attah: what do you mean? | 17:45 |
| attah | I built one version for arm: 150k, and the mandatory i486 package is 10x that | 17:46 |
| attah | and a lot of it in the binary | 17:46 |
| attah | (apps building with sfdk, that is) | 17:55 |
| mkolman | attah: check if the i486 version is stripped | 19:16 |
| mkolman | that could explain the difference | 19:16 |
| *** Ischwitch is now known as Ingvix | 21:22 | |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!