Submitting Kalendar for KDE Review

Tue Oct 26 00:09:40 BST 2021

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?

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
> > > >
> > >
> > >
> > >
> > >
> > >
> >
>
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20211026/95e7f436/attachment.htm>