| dcaliste | Hello chriadam, how are you ? | 06:59 |
|---|---|---|
| chriadam | hi dcaliste, I'm well thank you | 06:59 |
| chriadam | how has your week been? | 06:59 |
| dcaliste | It was great, thanks. | 07:01 |
| dcaliste | Didn't touch much code for Sailfish though ;) And I screwed up my SDK installation once again. Docker and myself are not good friends... | 07:01 |
| dcaliste | But maybe we can discuss a bit the different possibilities to support alarm attachments in mKCal, see https://git.sailfishos.org/mer-core/mkcal/merge_requests/65 | 07:04 |
| dcaliste | This MR is adding support for component attachments by adding a new table. Attachments were previously supported for URI only, and saved as a string list in the component table. | 07:05 |
| dcaliste | This is not convenient since attachment may be inline data and have additional parameters like the mime type. | 07:06 |
| dcaliste | Thus the addition of a new table. | 07:06 |
| dcaliste | From the iCal RFC, ATTACH line can be added to VALARMs also, not only components. | 07:07 |
| dcaliste | This is supported at the moment for URIs only by storing a string list in the alarm table. | 07:07 |
| dcaliste | For the same reason, this is not convenient. | 07:08 |
| dcaliste | There are three possibilities to add support for attachment for alarms : | 07:08 |
| dcaliste | - add now a alarmId column in the new attachment table, and depending if componentId or alarmId is not null, we know to which element the alarm is refering to. | 07:09 |
| dcaliste | - use negative values in attachment:componentId to refer to alarm ids and positive values to refer to component ids. | 07:10 |
| dcaliste | - create another table with the same layout as the attachments one but dedicated to alarm attachment. | 07:10 |
| chriadam | indeed. | 07:10 |
| chriadam | @flypig: did you get a chance to look at that one ^ | 07:10 |
| chriadam | my first impressions of the options are: | 07:11 |
| dcaliste | Honestly, I don't have any idea which solution is better in term of maintenance or good practices. | 07:11 |
| flypig | Sorry, I didn't I'm afraid. | 07:11 |
| chriadam | 1) I like that the columns are separate, so it's clear what each field means, without "magic". but I don't like that it will be difficult to enforce reference constraints (e.g. foreign key constraints on either componentId or alarmId ... tricky) | 07:12 |
| chriadam | 2) I like that it's simple. but I don't like "magic" e.g. just from looking at the db it's impossible ot tell that negative value means alarmId | 07:12 |
| chriadam | 3) duplicating the entire table schema seems suboptimal, but allows for maximum reference enforcement, I guess... | 07:12 |
| chriadam | pvuorela: did you have any thoughts about which might be best? | 07:13 |
| chriadam | I don't have strong feelings in any direction, honestly. | 07:13 |
| dcaliste | About reference enforcement, there is not any at all (no delete cascade and things like that) in mKCal. All is handled by hand in the C++ code. So, following the same spirit, it's not really an issue ;) | 07:14 |
| chriadam | indeed. | 07:14 |
| chriadam | although, one day, it might be nice to fix that.. | 07:14 |
| chriadam | I'm inclined to lean toward (1) for now | 07:15 |
| chriadam | flypig: do you have an off-the-cuff opinion? | 07:15 |
| flypig | Ah, sorry, I'd need a little time to catch up. | 07:16 |
| pvuorela | do we have any actual need for attachments in alarms? if not, could just skip? | 07:18 |
| dcaliste | 2) can be handled also easily without too much code duplication by playing a little bit with the preprocessor. The reading and writing part is already working on a prepared SQlite statement, so the duplication would just be when preparing the statement, which is two to three lines. In case it's better to have clearer schema, then 2) is not that much of a pain. | 07:18 |
| dcaliste | pvuorela, since we can resort on option 2) or 3), which means no modification to the schemas of the new attachments table, then we're safe indeed. | 07:19 |
| dcaliste | This discussion was more about "is this new attachment table future-proof", listing how alarm attachment could be added in the future without requiring a database migration. | 07:19 |
| pvuorela | though would be good if we had some real schema migration for the database instead of trying to fit new needs on the existing schema. | 07:21 |
| chriadam | well, data migration is unfortunately issue-prone | 07:21 |
| chriadam | better, if possible to avoid messy data migration steps, until schema breaks are needed, I suppose | 07:22 |
| pvuorela | on kf5-calendarcore the alarm attachments seem to have different api anyway, not using Attachment. | 07:22 |
| dcaliste | pvuorela, oh, nice point, I didn't noticed. | 07:22 |
| pvuorela | chriadam: true also, but in mkcal case we've never updated the schema. i have a feeling doing that on some point could have been better in the long run. | 07:23 |
| chriadam | yeah. | 07:23 |
| chriadam | ok, so where does that leave us? don't worry about trying to future-proof the schema of the new attachments table, currently? | 07:25 |
| dcaliste | So keep it as it is proposed is enough to support KCalendarCore attachment API. | 07:25 |
| pvuorela | sounds good | 07:27 |
| chriadam | something which affects all MRs more generally: apparently branching for 4.2.x has been delayed by a little while | 07:28 |
| chriadam | and considering how important that release is, my preference would still be to delay merging things, until after the branching | 07:28 |
| chriadam | but that is quite a long delay :-( | 07:28 |
| chriadam | i.e. another 10 days or so IIRC | 07:28 |
| dcaliste | About my other MRs, no problem with me. | 07:29 |
| dcaliste | Concerning this specific mKCal attachment stuff, I did it for people in OMP who needed it for internal reasons not disclosed ;) | 07:30 |
| dcaliste | So you should see with them if it is in a hurry for them or not. | 07:30 |
| chriadam | pvuorela: do you know if https://git.sailfishos.org/mer-core/nemo-qml-plugin-calendar/merge_requests/86 is urgent? | 07:30 |
| chriadam | hmm, no bug reference | 07:31 |
| pvuorela | that requires rethinking the whole architecture. | 07:31 |
| pvuorela | not going forward in a week or two. | 07:31 |
| dcaliste | Indeed ;) that's why I was proposing mKCal#65 | 07:31 |
| dcaliste | To help a bit avoiding to store string hashtable contents ! | 07:32 |
| pvuorela | yep, with mkcal pr it's at least possible to store attachments and expect to get same things back later. | 07:32 |
| dcaliste | So if it's not in a hurry neither, no problem to wait for 4.2 branching, even if it is getting late. | 07:32 |
| chriadam | what is the conclusion? are we fine to merge mkcal#65 before branching? low risk, and potentially unblocks OMP? | 07:33 |
| pvuorela | even though doing the attachments properly in qml plugin + app might not absolutely require the new parts. | 07:33 |
| pvuorela | chriadam: as that didn't appear too risky, i think we could merge it even earlier. i'll need to check it again though. | 07:34 |
| chriadam | ok. thank you | 07:34 |
| chriadam | I'll leave it in your hands. | 07:34 |
| dcaliste | pvuorela, I agree, duplicating the KCalendarCore::Attachment into nemo-qml-plugin-calendar can be done separatetly, assuming that the database is following. | 07:34 |
| dcaliste | pvuorela, chriadam, there is one thing about mKCal#65 that I would like to get your opinion on : attachment custom properties... | 07:35 |
| dcaliste | The RFC is saying that you can save as many custom properties as you want for each attachment. | 07:35 |
| dcaliste | This is not supported by KCalendarCore at the moment. | 07:35 |
| dcaliste | But if it is important for OMP, it could be added. | 07:36 |
| pvuorela | maybe i wouldn't prematurely add new features, could be that don't end up needed at all. | 07:36 |
| dcaliste | At the moment, MR#65 is not supporting it neither, but should we once again be future proof by adding a string list column in the new attachments table in case ? | 07:36 |
| dcaliste | And leave it not used at the moment. | 07:37 |
| pvuorela | dunno, until we add attachment usage we are are quite free to modify that part of the schema. | 07:38 |
| pvuorela | hey and doing the attachments properly. i'd maybe expect servicehandler.h api / plugins getting used. | 07:38 |
| sashikknox_ | lbt: can i get account for git.sailfishos.org?) | 07:40 |
| dcaliste | As long as the code is not released, because, as it is done at the moment, on each database opening, the attachment table is created. So we cannot add a modified schema after a released was done, even if we didn't actually used the attachment code in that release. | 07:41 |
| chriadam | sashikknox_: err | 07:41 |
| pvuorela | dcaliste: simple to add some migration that even just recreates the whole table if it detects it's the old version. | 07:42 |
| chriadam | sashikknox_: https://forum.sailfishos.org/t/migrating-sailfish-os-git-repos-to-github/6141/23 | 07:42 |
| chriadam | well, I mean the main post, not the comment I linked to, probably | 07:42 |
| dcaliste | That's true, but I'm not feeling good with table migration ;) Anyway, if you think it's allright like that, I trust your knowledge on this. | 07:43 |
| dcaliste | So, pvuorela, we just need to agree on the migration lines on attachment reading in the MR#65 and that will be good ! | 07:46 |
| dcaliste | (about supporting existing attachment URIs saved as string list) | 07:47 |
| pvuorela | since we haven't used for anything, i wouldn't bother much. | 07:47 |
| dcaliste | I would not myself neither, but just in case some users have some attachments as URIs that they don't want to disappear on the server side. But I agree that the use case is very restricted and maybe not worth the four or five lines in this already over long routine. | 07:49 |
| pvuorela | they would need quite a bit of custom code for that. and privileges given to the custom code to be able to even access the database. | 07:51 |
| chriadam | ok, so, concretely, does dcaliste need to make any changes to that MR#65 right now, or at this point should he just wait to see what review comments you have, pekka? | 07:51 |
| dcaliste | Well, I'm not speaking on device. I'm speaking of a use case were they have added some attachment as URIs on their desktop for instance and are happy with. The event synced to the phone. The event is then modified on device to change let say the start time and synced back to the desktop. Then, the URIs would disappear on desktop. Before the MR they would have not... | 07:53 |
| pvuorela | but do we have any sync code that touches the attachments at all? | 07:54 |
| dcaliste | chriadam, beside this minor migration point, I think we agree to keep the MR as it is, supporting attachment for components (but without custom properties). | 07:54 |
| dcaliste | pvuorela : yes we have. They are imported in the database and restored when exported into iCal format. | 07:54 |
| pvuorela | hm, right. | 07:54 |
| dcaliste | Actually, they are imported only if they are URIs, and all other properties like the mime type are lost. Thus the new table addition. | 07:55 |
| sashikknox_ | chriadam: need jusr wait when it available on github?) | 07:58 |
| chriadam | sashikknox_: my understanding is that you won't need to wait long at all. but perhaps I am mistaken. maybe lbt will still add new user accounts, but it seems unlikely at this point. | 07:59 |
| chriadam | of course, if you have some pressing need, I'm sure it can be accommodated | 07:59 |
| *** frinring_ is now known as frinring | 08:01 | |
| sashikknox_ | chriadam: i just want fixes fir SDL2 now ) https://forum.sailfishos.org/t/bug-4-1-0-24-sdl2-cant-create-eglsurface-cant-return-sdl-getdevicedpi/6654 | 08:01 |
| chriadam | lbt: ViGe: ^ not sure where the migration is at | 08:02 |
| chriadam | dcaliste: well, I guess some more discussion is needed on that MR related to the migration | 08:03 |
| dcaliste | Yes, we can continue with pvuorela on the MR comments. | 08:03 |
| chriadam | thanks very much. was there anything else to discuss today? | 08:04 |
| chriadam | still https://git.sailfishos.org/mer-core/mkcal/merge_requests/63 and https://git.sailfishos.org/mer-core/messagingframework/merge_requests/59 I suppose | 08:04 |
| chriadam | I will run qmf#59 unit tests this week | 08:04 |
| chriadam | for mkcal#63 I tend to think it can wait until after branching | 08:05 |
| dcaliste | Thanks chriadam for qmf#59 testing. | 08:05 |
| chriadam | aside from that i have no qualms with merging tha tone | 08:05 |
| dcaliste | about mkcal#63, it's more about cleaning code, so it can definitely wait after branching. | 08:06 |
| chriadam | thanks. | 08:06 |
| chriadam | any other changes which need our attention at this stage? or flypig / pvuorela did you have things to discuss today? | 08:06 |
| dcaliste | No, thanks, that's all from my side. | 08:07 |
| chriadam | ok, in that case, I hope you have a great week! thanks again for your time and efforts! it is much appreciated. | 08:08 |
| chriadam | good night :-) | 08:09 |
| * chriadam -> away | 08:09 | |
| dcaliste | Thanks, chriadam, pvuorela and flypig. Enjoy your week. | 08:10 |
| *** sashikknox[m] is now known as sashikknox | 08:20 | |
| lbt | sashikknox_: msg me your emal/username and I'll set it up - git is going away RSN but OBS is around and you may want to use that as it improves | 08:56 |
| ViGe | chriadam, sashikknox_: yeah, the git migration is happening really soon. But I wouldn't worry about it too much. In the worst case the MR just needs to be reopened on github, which is of course a little bit of extra work, but nothing compared to the actual implementation work. | 09:18 |
| *** frinring_ is now known as frinring | 19:29 | |
| nshiell | Hi I have a problem updating my phone to the latest SailfishOS | 20:34 |
| mal | nshiell: which device? | 20:39 |
| Nico-old-defunct | And what problem? | 20:40 |
| nshiell | An XA2 | 20:42 |
| nshiell | I goto Sailfish OS Updates | 20:42 |
| nshiell | When I try to download the update I see: | 20:42 |
| nshiell | Problem with Store | 20:42 |
| nshiell | Could not determine size of update | 20:43 |
| mal | do you have any patches or things from openrepos? | 20:53 |
| nshiell | A few | 21:27 |
| nshiell | The OKBoard keyboard | 21:28 |
| nshiell | I might have a few other things - apps basically | 21:28 |
| Nico-old-defunct | Try uninstalling the OKBoard temporarily at least | 21:35 |
| Nico-old-defunct | It sometimes breaks upgrades | 21:35 |
| Nico-old-defunct | There is also a "clear store data" in Sailfish utils, that may help? | 21:35 |
| nshiell | I'll give those ideas a shot | 21:36 |
| mal | I think release notes mentions some apps from openrepos which might cause issues | 21:41 |
| Nico-old-defunct | The list wasn't really up to date :D | 21:42 |
| nshiell | Removed OKBoard, rebooted, seems not to have made a difference | 21:42 |
| mal | Nico-old-defunct: it's very difficult to make a full list, those are just what were noticed during testing | 21:44 |
| Nico-old-defunct | mal: Some of those were wrong though, at least when I upgraded, but maybe the notes got updated later, since the comments called them out :3 | 21:46 |
| mal | Nico-old-defunct: it's possible the apps were fixed | 21:51 |
| Nico-old-defunct | Maybe | 21:52 |
| Nico-old-defunct | I thought the upgrade notes were just copied from the last release | 21:52 |
| nshiell | I have the Storeman app, I go into "Installed applications", I see a few things | 21:56 |
| nshiell | libpython3_7m1_0 looks interesting | 21:56 |
| nshiell | rrdtool | 21:57 |
| nshiell | gnuplot | 21:57 |
| nshiell | and a few other things | 21:57 |
| mal | have you updated all of those the latest versions? | 22:03 |
| nshiell | Can updating be done with the Storeman app? | 22:17 |
| nshiell | Does it do it automatically? | 22:18 |
| mal | I mean updating the apps from within storeman | 22:32 |
| oozbooz | are there field reports on xperia 10 II with the latest 64 bit image? | 22:38 |
| *** nyov is now known as Guest71608 | 22:42 | |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!