Submitting Kalendar for KDE Review

Albert Astals Cid aacid at kde.org
Tue Oct 26 21:16:10 BST 2021 

El dimarts, 26 d’octubre de 2021, a les 1:09:40 (CEST), Claudio Cambra va escriure:
> Hi Albert,
> 
> I forgot to hit the reply all button... thanks for ccing k-c-d back.
> 
> 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?

Please email release-team at kde.org about this :)

Cheers,
  Albert

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

More information about the kde-core-devel mailing list