| dcaliste | Good morning pvuorela, sorry for being late. | 08:28 |
|---|---|---|
| pvuorela | dcaliste: good morning. np. | 08:28 |
| dcaliste | Thank you for the quick reviews on mKCal PRs. | 08:31 |
| pvuorela | dcaliste: sure. good stuff there. have been checking them closely now. | 08:31 |
| pvuorela | tried your wip commit on nemo-calendar but unfortunately it's now failing even more on tst_calendarevent. | 08:32 |
| dcaliste | Really ? It was fine for me yesterday evening. I had some issues with agenda model test because of the Manager::storageModified signal. | 08:32 |
| dcaliste | Strange also, because the only change in the WIP commit regarding calendar event test is the getEventAttendee one. | 08:35 |
| pvuorela | eventSpy.wait() failing there quite many times. | 08:36 |
| dcaliste | This one is not necessary a good fix by the way. The issue at hand is this one : | 08:36 |
| dcaliste | (eventSpy failing but not always looks like a race condition, I'll investigate keeping this in mind) | 08:36 |
| dcaliste | - in calendarworker.cpp#377, an organizer is created and his email is set, but not its name. | 08:37 |
| dcaliste | - so the event organizer.email() is not null, but organizer.name() is. | 08:38 |
| dcaliste | - in mkcal, with a recent fix, the organizer is stored with the corresponding attendee details, including the name. | 08:38 |
| dcaliste | - so after reread of the database, the organizer has a name. | 08:39 |
| dcaliste | - but now, with the updated signal, there is no round trip through the database, and the organizer is left without name. | 08:39 |
| dcaliste | - then in getEventAttendee, an attendee is deduplicated from the organizer by testing name and email, see around calendarutils.cpp#365 | 08:40 |
| dcaliste | - so if the organizer has no name, like in the test, it is not properly deduplicated from the corresponding attendee and this result in the attendee list containing both the organozer as attendee and the organizer with no name so 4 attendees instead of 3 in the test(). | 08:42 |
| dcaliste | In the WIP commit, I'm mimiccing the mKCal behaviour by restoring the organizer with its attendee description. Not sure it's the right way to do it... | 08:43 |
| pvuorela | uhm, ok. | 08:45 |
| pvuorela | perhaps in longer term we should some day separate organizer from the chair role. | 08:45 |
| dcaliste | Yes, at the moment, I try not to induce regressions ! | 08:47 |
| dcaliste | About the eventSpy failing, can you tell me where they are ? | 08:47 |
| dcaliste | I've tried again on JollaC and it's not failing. I guess it's because the proc is slower than on more recent device. | 08:48 |
| dcaliste | The immediate possibility I see is that the signal is emitted before we enter the wait() loop. | 08:48 |
| pvuorela | tst_calendarevent.cpp(228) tst_calendarevent.cpp(303) tst_calendarevent.cpp(360) tst_calendarevent.cpp(612) tst_calendarevent.cpp(651) tst_calendarevent.cpp(780) | 08:48 |
| dcaliste | Ok, not the case for 228. There is a timer of 5ms in the manager if I remember correctly that should ensure that we have time to enter the waiting loop before doing anything else. | 08:50 |
| dcaliste | How are you running the test ? I'm doing myself "rm -f db3; SQLITESTORAGEDB=db3 ./tst_calendarevent testSave" | 08:51 |
| dcaliste | the changed branch of mKCal being compiled and installed on device before running this. | 08:52 |
| pvuorela | testrunner-lite -f /opt/tests/nemo-qml-plugins-qt5/calendar/tests.xml -o nemocal | 08:52 |
| dcaliste | Ok, let me try this one... | 08:53 |
| pvuorela | hm, not sure what on the wip commit could break it so much. | 08:54 |
| pvuorela | i'll try rebuilding to rule out any funny stuff. | 08:55 |
| dcaliste | Do you know where is blts-tool ? It seems to be required to run the tests via testrunner-lite | 08:58 |
| pvuorela | dcaliste: it's qa extra repository. need to be added manually. | 09:00 |
| pvuorela | not entirely sure what the repository should be for you. | 09:00 |
| pvuorela | hey but anyhow, tried now rebuilding and reinstalling and the tests passed. sorry for the hassle. think i might have had bad mkcal version. | 09:01 |
| dcaliste | Ah, that's also a good reason ! | 09:01 |
| pvuorela | the sources here if it's any help. https://github.com/mer-qa/blts-tools/ | 09:02 |
| dcaliste | I need to add the right version dependency in spec file to avoid this to happen. | 09:02 |
| pvuorela | i was thinking of bumping mkcal version to 0.6.0 with these update signal adjustments. | 09:03 |
| dcaliste | Thanks for blts-tool. I'll compile it and install it. It may provide some slightly different environment for tests. | 09:03 |
| dcaliste | Ok, so I'll put that version requirement in QML binding spec file. | 09:04 |
| pvuorela | on that organizer adjustment. i'm guessing it could be fine to prioritize the attendee event details for now but that could perhaps have some rationale mentioned. | 09:04 |
| pvuorela | the calendarworker error output has a small typo | 09:04 |
| dcaliste | Right, I'll drop a comment there. | 09:04 |
| pvuorela | uneanble | 09:04 |
| dcaliste | Indeed, I've just noticed it myself too. I'll fix it also. Thanks. | 09:05 |
| pvuorela | and the final part on tst_calendarmanager seems sensical so with those small adjustments it could be all ok. | 09:05 |
| dcaliste | There are the issues with agenda model. | 09:06 |
| dcaliste | Waiting on CalendarManager::storageModified to ensure that events are saved is not a possibility anymore in the initTestCase. | 09:07 |
| dcaliste | I need to check if that's the root problem of the issues reported later not finding CalendarStoredEvent. | 09:07 |
| pvuorela | hm, ok. | 09:08 |
| pvuorela | so did you get some failure yourself? | 09:08 |
| dcaliste | I'll try to sort this out this morning after the meeting. | 09:08 |
| dcaliste | Yes it's failing with a lot of warnings like : | 09:09 |
| dcaliste | CalendarStoredEvent* CalendarManager::eventObject(const QString&, const QDateTime&) No event with uid "7D995F1F-FE55-409B-9EC6-0D563A00726F" | 09:09 |
| pvuorela | right. | 09:09 |
| dcaliste | After this is fixed and approved, I'll rebase https://github.com/sailfishos/nemo-qml-plugin-calendar/pull/30 on top. | 09:12 |
| pvuorela | sounds good. | 09:13 |
| dcaliste | The changes are extensive of course, so take your time for the review. As I mentioned in the PR message, I've separated the work into several smaller commits, where tests should be ok at each step. Maybe it could be easier like that. | 09:14 |
| dcaliste | But sometimes it imposed to be a bit silly by switching several time back and forth between CalendarData::Event and KCalendarCore::Incidence::Ptr. | 09:15 |
| dcaliste | Because this or that part was not yet ported. | 09:15 |
| dcaliste | Ok, pvuorela, I've fixed the agenda model test and squashed the commits (update the mkcal spec dependency, added a comment in getEventAttendee and correct the mispell). | 09:37 |
| dcaliste | Tests are passing for me now. | 09:37 |
| pvuorela | still a typo in the warning :) | 09:39 |
| dcaliste | The problem with agenda model (and maybe some others) was related to the fact that the calendarworker sent list was reset on storage modification, while the corresponding list in manager was reset after data load. | 09:39 |
| dcaliste | Arg. | 09:39 |
| dcaliste | Fixed ! | 09:41 |
| pvuorela | tests still passed. i'll check the thing still with thought and hopefully merge it later. | 09:43 |
| dcaliste | Sure thank you. | 09:45 |
| pvuorela | dcaliste: suppose we're done. could go get some lunch. after that think i could start merging things if nothing new still pops up. | 10:04 |
| dcaliste | Ok, that's great. Thank you. Enjoy your meal. | 10:04 |
| pvuorela | thanks. and for the PRs! | 10:05 |
| poetaster | rinigus: how to I specify noarch on obs correctly. Had BuildArch: noarch ina spec. | 13:30 |
| nephros-nc-bridge | [nctalk] <nephros> poetaster: that should be correct. | 14:37 |
| poetaster | nephros-nc-bridge, I was a bit thrown that obs built it for all targets and arches | 14:57 |
| poetaster | nephros-nc-bridge, trying to reduce cruft since I have a number of 'noarch' needlessly wasting space | 14:58 |
| poetaster | nephros-nc-bridge, perhaps a <repository><arch>noarch</arch> clause? | 14:59 |
| rinigus | poetaster: in this respect, you would get that noarch app built for all target-arch combinations. don't worry, that's the way OBS is | 16:08 |
| nephros-nc-bridge | [nctalk] <nephros> poetaster, rinigus: in the case the whole app is noarch (not just a sub package), I usually just disable all repos in that package and build against latest. | 16:19 |
| nephros-nc-bridge | [nctalk] <nephros> Chum obviously needs all enabled though | 16:19 |
| nephros-nc-bridge | [nctalk] <nephros> rinigus: about that obs policy: "no binary source packages": I dabble in Kodi building, and that needs java/jre to build. Is it a terrible crime to put a IcedTea JRE tarball in the sources? | 16:21 |
| rinigus | nephros: this is a good question! that binary is used just for building, right? what would be its arch? is it possible to make it useful on devices? | 16:48 |
| rinigus | I am on gentoo myself and we have few binary packages on it, not all is from source. maybe we should approach it in a similar manner... In principle, we should be able to have exceptions when we all know why these exceptions are needed | 16:50 |
| rinigus | cc piggz ^ | 16:50 |
| nephros-nc-bridge | [nctalk] <nephros> rinigus: I doubt people need a JRE packaged for use but I'm NOT going to try building it from source ;) | 16:55 |
| nephros-nc-bridge | [nctalk] <nephros> In the case of kodi its needed to generate some files, none of java or the jre ends up in the built package. | 16:55 |
| nephros-nc-bridge | [nctalk] <nephros> ATM I'm using the Gentoo binary tarball. | 16:56 |
| rinigus | nephros: but that jre is which arch? | 16:56 |
| nephros-nc-bridge | [nctalk] <nephros> arm at the moment. I think it works on aarch64 too. | 16:56 |
| nephros-nc-bridge | [nctalk] <nephros> https://build.sailfishos.org/package/show/home:nephros:devel:kodi/kodi | 16:58 |
| nephros-nc-bridge | [nctalk] <nephros> But the aarch64 build is stuck in DoD at the moment. | 16:59 |
| rinigus | Keto and lbt_: see above, DoD packages reported above | 17:44 |
| rinigus | nephros: it looks fine by me. let's see if it works for aarch64. in principle, you can add then jre for every arch and then use accordingly | 17:46 |
| Keto | I don't know whats wrong with the DoD | 17:59 |
| *** marja is now known as Guest2247 | 18:26 | |
| rasp | Ok i'm back | 18:42 |
| rasp | Got an xperia on the way cause i sold out | 18:42 |
| nephros-nc-bridge | [nctalk] <nephros> rinigus: thanks for checking and confirming. Don't want to break too many rules :) | 18:47 |
| attah | poetaster: so, what am i installing? | 18:53 |
| rinigus | nephros: I think it makes sense. We would just have to verify somehow the binaries that are included in this manner. | 19:11 |
Generated by irclog2html.py 2.17.1 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!