<div dir="ltr"><div>Hi Albert,<br></div><div><br></div><div>I forgot to hit the reply all button... thanks for ccing k-c-d back.<br></div><div><br></div><div>Now that the two-week review period is over, I wanted to ask if it is possible for Kalendar to be included in KDE Gear 21.12?</div><div><br></div><div>I'd also like to ask what your thoughts would be on having a "beta" or "pre-release" release at the end of next week? This could really help us gather bug reports and patch up as much stuff as possible before December.</div><div><br></div><div>Thanks for your help!</div><div><br></div><div>Clau<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 18, 2021 at 10:51 AM Albert Astals Cid <<a href="mailto:aacid@kde.org">aacid@kde.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I think you may have dropped k-c-d from the CC, adding it back.<br>
<br>
<br>
El dissabte, 16 d’octubre de 2021, a les 14:18:08 (CEST), Claudio Cambra va escriure:<br>
> Hi Albert,<br>
> <br>
> Thanks for your thorough review, and sorry about the late reply. We've been<br>
> working on addressing these issues and have fixed them:<br>
> <br>
> - Q_PROPERTY compilation warnings are now fixed<br>
> - persistentIndex in the todomodel class are now QModelIndex<br>
> - We now provide vendored copies of the components we require from Kirigami<br>
> Addons (for now, until KA passes KDE Review)<br>
> - We added KLocalizedString::setApplicationDomain("kalendar");<br>
> - We now use standaloneMonthName for our month-year strings per your<br>
> recommendation<br>
> - We fixed the issue with valgrind you brought up<br>
> <br>
> Additionally, we went ahead and addressed the user-facing issues you<br>
> commented on (thanks!):<br>
> - You should now get a pointing-hand cursor when hovering over links in the<br>
> incidence information drawer<br>
> - Clicking on empty space in the various views should now deselect the<br>
> current incidence and close the incidence information drawer<br>
> <br>
> Once again, thank you for taking the time to review Kalendar -- if you find<br>
> anything else we should fix, we will be very happy to do so!<br>
<br>
Good work!<br>
<br>
Cheers,<br>
  Albert<br>
<br>
> <br>
> Clau<br>
> <br>
> On Mon, Oct 11, 2021 at 12:01 AM Albert Astals Cid <<a href="mailto:aacid@kde.org" target="_blank">aacid@kde.org</a>> wrote:<br>
> <br>
> > El diumenge, 10 d’octubre de 2021, a les 22:45:06 (CEST), Claudio Cambra<br>
> > va escriure:<br>
> > > Hey all,<br>
> > ><br>
> > > We would like to move Kalendar to KDE review. We would also like to ask<br>
> > if<br>
> > > it would be possible to be release along with KDE Gear since the KDE<br>
> > > PIM libraries don't have a stable ABI, and we need to make sure we<br>
> > > remain compatible with them.<br>
> > ><br>
> > > For those who don't know, Kalendar is a new calendar application built<br>
> > > with Kirigami and Akonadi. Our repo is located at<br>
> > > <a href="https://invent.kde.org/pim/kalendar/" rel="noreferrer" target="_blank">https://invent.kde.org/pim/kalendar/</a>. We welcome any feedback!<br>
> ><br>
> > These seem like important enough to fix<br>
> ><br>
> > AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/hourlyincidencemodel.h:30:<br>
> > Warning: Property declaration model has no READ accessor function or<br>
> > associated MEMBER variable. The property will be invalid.<br>
> > AutoMoc:<br>
> > /home/tsdgeos/devel/kde/kalendar/src/incidenceoccurrencemodel.h:42:<br>
> > Warning: Property declaration filter has no READ accessor function or<br>
> > associated MEMBER variable. The property will be invalid.<br>
> > AutoMoc: /home/tsdgeos/devel/kde/kalendar/src/multidayincidencemodel.h:35:<br>
> > Warning: Property declaration model has no READ accessor function or<br>
> > associated MEMBER variable. The property will be invalid.<br>
> > AutoMoc:<br>
> > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:17:<br>
> > Warning: Property declaration incidenceChanger has no READ accessor<br>
> > function or associated MEMBER variable. The property will be invalid.<br>
> > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:18:<br>
> > Warning: Property declaration calendar has no READ accessor function or<br>
> > associated MEMBER variable. The property will be invalid.<br>
> > /home/tsdgeos/devel/kde/kalendar/src/todosortfilterproxymodel.h:19:<br>
> > Warning: Property declaration filter has no READ accessor function or<br>
> > associated MEMBER variable. The property will be invalid.<br>
> ><br>
> ><br>
> ><br>
> ><br>
> > This is also interesting<br>
> ><br>
> > /home/tsdgeos/devel/kde/kalendar/src/todomodel.cpp:168:39: warning: loop<br>
> > variable ‘persistentIndex’ of type ‘const QPersistentModelIndex&’ binds to<br>
> > a temporary constructed from type ‘const QModelIndex’<br>
> > [-Wrange-loop-construct]<br>
> >   168 |     for (const QPersistentModelIndex &persistentIndex :<br>
> > persistentIndexes) {<br>
> >         m_persistentIndexes << persistentIndex; // Stuff we have to update<br>
> > onLayoutChanged<br>
> ><br>
> ><br>
> > You're pretending to keep a list of persistent model indexes, but you're<br>
> > not, because m_persistentIndexes is a QModelIndexList that is a<br>
> > QList<QModelIndex> so you're losing the "persistence" of it.<br>
> ><br>
> > So you either<br>
> ><br>
> >   A) don't need the persistance of it all if this works and<br>
> > persistentIndex should just be a QModelIndex<br>
> > or<br>
> >   B) you should make m_persistentIndexes a list of QPersistentModelIndex<br>
> > (and remove the & from the persistentIndex declaration to make the warning<br>
> > go away).<br>
> ><br>
> ><br>
> ><br>
> > Aaaaaaaaaaaaaand I can't run it ^_^<br>
> ><br>
> > QQmlApplicationEngine failed to load component<br>
> > qrc:/main.qml:663:9: Type TodoPage unavailable<br>
> > qrc:/TodoPage.qml:168:17: Type TodoTreeView unavailable<br>
> > qrc:/TodoTreeView.qml:9:1: module "org.kde.kirigamiaddons.treeview" is not<br>
> > installed<br>
> ><br>
> > What provides org.kde.kirigamiaddons.treeview ?<br>
> > Ok kirigami-addons, this is a bit "problematic" because it is still listed<br>
> > under unstable in our download section<br>
> > <a href="https://download.kde.org/unstable/kirigami-addons/0.2/" rel="noreferrer" target="_blank">https://download.kde.org/unstable/kirigami-addons/0.2/</a><br>
> > Having something stable depend on something unstable is not "good".<br>
> ><br>
> ><br>
> ><br>
> > You're missing a KLocalizedString::setApplicationDomain("kalendar"); just<br>
> > after your QApplication<br>
> ><br>
> ><br>
> > I get a<br>
> > "0 instead of 1 arguments to message {Ends on %1} supplied before<br>
> > conversion."<br>
> > when selecting one of my existing events<br>
> ><br>
> ><br>
> > You can't use "<b>MMMM</b> yyyy" to get month and year because for some<br>
> > languages "MMMM" is translated as "of monthname" so that the typical DD<br>
> > MMMM yyyy looks fine for them and you get "25 of Janaury of 2023", but then<br>
> > if you do "MMMM yyyy" you get "of monthname 2021" which is wrong, so my<br>
> > suggestion is do something like i18nc("%1 is month name, %2 is year", "%1<br>
> > %2", Locale.standaloneMonthName(bla), blo));<br>
> ><br>
> ><br>
> ><br>
> > Running with valgrind i got<br>
> > ==42992== Invalid read of size 16<br>
> > ==42992==    at 0x142A6C69: ???<br>
> > ==42992==    by 0x191F6C17: ???<br>
> > ==42992==  Address 0x191f6c1e is 30 bytes inside a block of size 42 alloc'd<br>
> > ==42992==    at 0x483E7C5: malloc (vg_replace_malloc.c:380)<br>
> > ==42992==    by 0x7B2E7D2: QArrayData::allocate(unsigned long, unsigned<br>
> > long, unsigned long, QFlags<QArrayData::AllocationOption>) (in<br>
> > /usr/lib/libQt5Core.so.5.15.2)<br>
> > ==42992==    by 0x7BA967A: QString::fromLatin1_helper(char const*, int)<br>
> > (in /usr/lib/libQt5Core.so.5.15.2)<br>
> > ==42992==    by 0x1C68FE: QString::QString(QLatin1String) (qstring.h:1067)<br>
> > ==42992==    by 0x1F1886:<br>
> > AttendeeStatusModel::AttendeeStatusModel(QObject*) (attendeesmodel.cpp:33)<br>
> > ==42992==    by 0x1F223C: AttendeesModel::AttendeesModel(QObject*,<br>
> > QSharedPointer<KCalendarCore::Incidence>) (attendeesmodel.cpp:78)<br>
> > ==42992==    by 0x1E2F2E: IncidenceWrapper::IncidenceWrapper(QObject*)<br>
> > (incidencewrapper.cpp:15)<br>
> > ==42992==    by 0x1BC3E8:<br>
> > QQmlPrivate::QQmlElement<IncidenceWrapper>::QQmlElement()<br>
> > (qqmlprivate.h:139)<br>
> > ==42992==    by 0x1BC435: void<br>
> > QQmlPrivate::createInto<IncidenceWrapper>(void*) (qqmlprivate.h:166)<br>
> > ==42992==    by 0x5674D62: QQmlType::create(QObject**, void**, unsigned<br>
> > long) const (in /usr/lib/libQt5Qml.so.5.15.2)<br>
> > ==42992==    by 0x56C6F1D: QQmlObjectCreator::createInstance(int,<br>
> > QObject*, bool) (in /usr/lib/libQt5Qml.so.5.15.2)<br>
> > ==42992==    by 0x56C7675: QQmlObjectCreator::create(int, QObject*,<br>
> > QQmlInstantiationInterrupt*, int) (in /usr/lib/libQt5Qml.so.5.15.2)<br>
> ><br>
> > When opening an event<br>
> ><br>
> ><br>
> ><br>
> > Now "as a user" comments, feel free to ignore:<br>
> >  * It feels weird that the urls on the description field of my events is<br>
> > clickable but i get no "pointy-hand" mouse icon on hovering over them<br>
> >  * Same for the attendees to a meeting<br>
> >  * I need a "add default reminder" option<br>
> >  * It's weird that you can't unselect events clicking on empty space<br>
> ><br>
> > Good start!<br>
> ><br>
> > Cheers,<br>
> >   Albert<br>
> ><br>
> > ><br>
> > > Thanks,<br>
> > > Clau<br>
> > ><br>
> ><br>
> ><br>
> ><br>
> ><br>
> ><br>
> <br>
<br>
<br>
<br>
<br>
</blockquote></div>