| dcaliste | Hello pvuorela, how are you ? | 08:00 |
|---|---|---|
| pvuorela | dcaliste: hey. good here. how are you? | 08:01 |
| dcaliste | I'm ok thank you. You had time last week to merge the factorisation into a base class last week. That's great. Thank you. | 08:03 |
| pvuorela | cheers. on the next ones the exception seemed fine but didn't have yet time to fully go through. | 08:04 |
| pvuorela | neither didn't yet get much into the mkcal change signalling but superficially it seemed good, i think. | 08:05 |
| dcaliste | Good, good. For the signaling stuff, you'll see that I'm exposing lists of touched incidences in the DB in the new signal. But actually, the QML plugin is not using them at the moment. | 08:06 |
| dcaliste | I think it's safer to suppose that we may find a way to use them at one point and already expose these lists in the API. | 08:06 |
| pvuorela | alright | 08:06 |
| dcaliste | We have them internally anyway, so they are not a burden to be built. | 08:07 |
| dcaliste | I tried to create a test to ensure viability of the approach, but I got an issue: | 08:07 |
| dcaliste | - testing the in-process new signaling is fine and is working as expected, but | 08:08 |
| dcaliste | - I was not able to test the out-of-process signaling. | 08:08 |
| dcaliste | I mean, when an external process is touching the DB and the storageModified is emitted. | 08:08 |
| dcaliste | If I try to do a modification from another thread, SQlite complains and don't want to proceed because it wants only one connection per process internaly, | 08:09 |
| dcaliste | And when I try to fork the test process for the child to touch the DB, Qt testing framework is messed up because it is reacting to the child SIGCHLD signal... | 08:10 |
| dcaliste | For the moment, I gave up, but if you have an idea or know how to use Qt testing framework with multi process, I would be grateful ;) | 08:11 |
| pvuorela | hm, could expect it should be possible to start external processes from the tests. though if there's a problem, perhaps it could be also something getting launched and stopped from the tests.xml pre/post steps. | 08:15 |
| dcaliste | Maybe it's possible to run QProcess in the test framework, but I was a bit hesitant to create an executable just to run two lines of code (storage->open(); storage->save()). For something that run externally and is run by hand outside the test itself, it's a bit complicated also because one need a bit of synchronisation, so the DB is touched after the test has run storage->open(). | 08:19 |
| pvuorela | yea, small executable though if it does the trick. | 08:23 |
| dcaliste | Indeed, I'll try with a QProcess to see if it's doing the job. That would help to have a complete coverage testset for signaling. | 08:23 |
| dcaliste | Ah, I forgot : I've just pushed a modification to the dissociation PR. I noticed two minor regression after the factorisation PR: | 08:37 |
| dcaliste | - syncFailureResolution() getter was redefined in CalendarEventModification, so I removed it, | 08:38 |
| dcaliste | - when I copied all the conversion CalendarData::Event <-> KCalendarCore::Incidence code into calendarutils.cpp, it seemed I used an older version than the latest one (the one with the sync resolution PR) where the sync failure got another entry : "upload-new". So I patched it. | 08:39 |
| dcaliste | Do you prefer a separate commit or PR for these minor fixes ? | 08:39 |
| pvuorela | fixing previous things could warrant a separate commit but i can live either way. | 08:42 |
| dcaliste | I can separate it into another commit and create a quick separated PR, no problem. | 08:43 |
| dcaliste | So, here it is : https://github.com/sailfishos/nemo-qml-plugin-calendar/pull/29 | 08:49 |
| dcaliste | I'll rebase the dissociation PR on master after the fixup PR will be approved and merged. | 08:49 |
| pvuorela | oh, the old pr could have been ok for that too. just the separate commit. | 08:51 |
| pvuorela | but whichever way :) | 08:51 |
| dcaliste | About the new dissociation code : we have a possibility that was not exploited before in the UI : when the dissociation fails for whatever buggy reason, the save was silently ignored. | 08:53 |
| dcaliste | Now, we may know that the dissociation failed. Should we just ignored it as before, or should we warn the user ? | 08:53 |
| dcaliste | The latter looks nicer, but it is raising the fact that the use cannot do anything about it, and it's an internal bug anyway. | 08:54 |
| pvuorela | keeping ignored wouldn't be a regression and if it's a thing that shouldn't happen. though it shouldn't hurt either if we can do some small warning with small amount of code. depends. | 08:56 |
| dcaliste | Currently, the dissociation happens when the user is validating the dialog, which is suboptimal to warn the user because he has done all his modification already and the page is transitioning. But now, it's possible to actualy move the dissociation code to the caller and don't push the modification dialog if it fails. But it means larger changes in the code. | 09:00 |
| pvuorela | if it's larger changes, perhaps keep then ignoring for starters :) | 09:05 |
| dcaliste | I agree. In particular, it's really a corner case that should not happen, and as far as I remember, I've never seen any complains on TJC or the forum about such case. | 09:06 |
| dcaliste | Ah, pvuorela, sorry, I forgot to ask : I've got issue pushing to jolla-calendar repository recently. I've checked that web access is still showing write permission for my login, but pushing there (force push or new branch) fails with "remote: Permission denied to create branch contributor-test" for instance. | 09:25 |
| dcaliste | Or "remote: Permission denied to update branch contributor-dissociate." when force pushing to an existing branch I'm the owner of. | 09:26 |
| dcaliste | Was there a change on branch naming convention for contribution by any chance ? | 09:26 |
| pvuorela | dcaliste: hmh. checking. | 09:28 |
| pvuorela | dcaliste: in the meanwhile: the latest nemo calendar pr merged. | 09:36 |
| dcaliste | Great, thanks. | 09:36 |
| dcaliste | I've just rebased and push the dissociation PR so it's not containing these fixes by itself. | 09:39 |
| pvuorela | dcaliste: for branches got response that those got unified as "contrib-*" | 09:40 |
| dcaliste | Ok, I'll then close and reopen the pending PR in jolla-calendar and use the contrib-* convention. No problem. | 09:40 |
| Keto | yep, sorry I forgot to mention that | 09:41 |
| dcaliste | Keto, no problem. | 09:43 |
| dcaliste | I've close the PR#314 there and opened a new one based on the new branch. | 09:44 |
| poetaster | rinigus, piggz storeman packages have been submitted to chum based on the new repos. please add mentaljam and olf as maintainers. | 09:44 |
| dcaliste | I'll do this for the week-vew PR soon also. | 09:44 |
| pherjung | Keto: I took a look at MeetBot/Limnoria this morning, but I failed to do something :( | 10:20 |
| pherjung | I'm not sure to understand correctly inFilter. As it is explained in the documentation, it's about communication with the bot | 10:21 |
| Keto | pherjung: and? | 10:49 |
| Keto | I mean, what part there is unclear? | 10:52 |
| pherjung | inFilter | 10:53 |
| pherjung | I think I misunderstood something | 10:54 |
| pherjung | Using inFilter will the plugin intercept all the message automatically, or have the user have to message the bot? | 10:56 |
| Keto | it will intercept all messages, and needs to check if the message is coming from the bridge user. and if it is, then do the nick replacement | 10:58 |
| pherjung | ok, I'll took a look this night. If I get something working I'll push to GitHub | 11:01 |
| Keto | ok, thanks | 11:01 |
| piggz | poetaster: rinigus: all those storeman versions? are they nescessary given that OBS will build for each release? | 14:00 |
| poetaster | piggz: it's still odd, but each package precludes sfos versions it's not aimed at. I don't like it. | 14:19 |
| poetaster | piggz: but I have to either convince the others that > 4.x sharing being removed or we lose support for < 4 | 14:20 |
| poetaster | piggz, my aim would be to move to one branch/master but it's early days. | 14:20 |
| poetaster | piggz, I also believe that 3.2 could go. It is now 'the state' that mentaljam had had it for supporting the installer. | 14:21 |
| poetaster | piggz, olf writes: https://github.com/storeman-developers/harbour-storeman/wiki/Delta-between-release-branches-and-master | 14:22 |
| rinigus | poetaster: can't you somehow organize it in a way where single master is pulled and options enabled/disabled depending on SFOS version? | 17:54 |
| rinigus | right now it is a pain to accept and start enabling/disabling all the sfos/arch combinations | 17:55 |
| poetaster | rinigus, I made that point to olf but he was a bit insistent about the 'spatial' separation. | 20:35 |
| poetaster | rinigus, I've been arguing for removing > 4 only (sharing) stuff to get back to a single master. | 20:36 |
| poetaster | rinigus, but I'll see if it can been done with conditionals | 20:37 |
| rinigus | poetaster: should be possible with the conditionals. good luck! | 20:38 |
| poetaster | rinigus, I believe it could be just a Share3.qml vs. Share4.qml thing. On it. | 20:41 |
| poetaster | rinigus, I must admit, this is rather a waste of time. the sfos3.3 version and it's sharing implementation are serviceable. | 21:13 |
| poetaster | I let myself get talked into things. sigh. | 21:15 |
| *** jules[m] is now known as julesOld[m] | 22:14 | |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!