Alligator in KDEReview
Hi all, I've moved Alligator to KDEReview. Alligator is an RSS feed reader I started developing because of the need for a feed reader for plasma mobile (and to waste some time during corona). Since it's built using kirigami, it's usable on both mobile and desktop. I've also ported it to android, where it works quite nicely. It still has some problems with correctly rendering some feeds because of the limited html & css subset supported by the QML label, but I'm working around those. If you find feeds that don't render nicely (for example where images are not limited to the page width), feel free to send them my way. If you want to try it, you can get android and flatpak builds from the binary factory. For releases, i plan on getting it into release service in time for 20.12. Before that, I'll probably release a beta version Since I'm still a beginner at programming with Qt/cpp/qml, I'd appreciate comments or suggestions on my code ;) Thanks, Tobias https://invent.kde.org/plasma-mobile/alligator https://binary-factory.kde.org/job/Alligator_android/ https://binary-factory.kde.org/job/Alligator_x86_64_flatpak/
Re: Alligator in KDEReview
El dijous, 29 d’octubre de 2020, a les 0:34:30 CET, Tobias Fella va escriure: > Hi all, > > I've moved Alligator to KDEReview. > > Alligator is an RSS feed reader I started developing because of the need > for a feed reader for plasma mobile (and to waste some time during > corona). Since it's built using kirigami, it's usable on both mobile and > desktop. I've also ported it to android, where it works quite nicely. > > It still has some problems with correctly rendering some feeds because > of the limited html & css subset supported by the QML label, but I'm > working around those. Any reason you're not using WebEngineView ? > If you find feeds that don't render nicely (for > example where images are not limited to the page width), feel free to > send them my way. > > If you want to try it, you can get android and flatpak builds from the > binary factory. > > For releases, i plan on getting it into release service in time for > 20.12. Before that, I'll probably release a beta version I'm not convinced you're in time for that, technically one could argue that you can join up to November 12 that is when we have the Feature Freeze, but for me it makes sense to only release things that are ready on Nov 5, which is really soon. https://community.kde.org/Policies/Application_Lifecycle says 2 weeks of kdereview. I don't want to be the bad cop, but i'd say you're 1 week late. > > Since I'm still a beginner at programming with Qt/cpp/qml, I'd > appreciate comments or suggestions on my code ;) You're missing KLocalizedString::setApplicationDomain in your main.cpp You're not deleteing the Author you create on Entry::Entry, i'm guessing a qDeleteAll(m_authors); in the destructor is what you want. Similarly you seem to also need a qDeleteAll(m_entries) in EntriesModel destructor clazy reports a few missing const & in your args, not that it really matters, but it'll be 0.001% faster which means saving 0.001% battery if you're using it in a phone :D https://paste.debian.net/1169487/ This animation feels super weird, is this something you can control or Kirigami provided? https://i.imgur.com/rleJfso.gif Cheers, Albert > > Thanks, > > Tobias > > > https://invent.kde.org/plasma-mobile/alligator > > https://binary-factory.kde.org/job/Alligator_android/ > > https://binary-factory.kde.org/job/Alligator_x86_64_flatpak/ > >
Re: Alligator in KDEReview
On 01.11.20 12:29, Albert Astals Cid wrote: El dijous, 29 d’octubre de 2020, a les 0:34:30 CET, Tobias Fella va escriure: Hi all, I've moved Alligator to KDEReview. Alligator is an RSS feed reader I started developing because of the need for a feed reader for plasma mobile (and to waste some time during corona). Since it's built using kirigami, it's usable on both mobile and desktop. I've also ported it to android, where it works quite nicely. It still has some problems with correctly rendering some feeds because of the limited html & css subset supported by the QML label, but I'm working around those. Any reason you're not using WebEngineView ? I'm not using Webengine because i want it to work on Android and using WebView I didn't get nice results due to other problems If you find feeds that don't render nicely (for example where images are not limited to the page width), feel free to send them my way. If you want to try it, you can get android and flatpak builds from the binary factory. For releases, i plan on getting it into release service in time for 20.12. Before that, I'll probably release a beta version I'm not convinced you're in time for that, technically one could argue that you can join up to November 12 that is when we have the Feature Freeze, but for me it makes sense to only release things that are ready on Nov 5, which is really soon. https://community.kde.org/Policies/Application_Lifecycle says 2 weeks of kdereview. I don't want to be the bad cop, but i'd say you're 1 week late. Yes I realized that after sending the email... Next release after that it is, then Since I'm still a beginner at programming with Qt/cpp/qml, I'd appreciate comments or suggestions on my code You're missing KLocalizedString::setApplicationDomain in your main.cpp You're not deleteing the Author you create on Entry::Entry, i'm guessing a qDeleteAll(m_authors); in the destructor is what you want. Similarly you seem to also need a qDeleteAll(m_entries) in EntriesModel destructor clazy reports a few missing const & in your args, not that it really matters, but it'll be 0.001% faster which means saving 0.001% battery if you're using it in a phone :D https://paste.debian.net/1169487/ Will fix This animation feels super weird, is this something you can control or Kirigami provided? https://i.imgur.com/rleJfso.gif I wouldn't call that an animation, it's just the page closing and a new one opening I can probably make that nicer by changing the content of the current page instead of replacing it. Thanks for your review, Tobias Cheers, Albert Thanks, Tobias https://invent.kde.org/plasma-mobile/alligator https://binary-factory.kde.org/job/Alligator_android/ https://binary-factory.kde.org/job/Alligator_x86_64_flatpak/
Re: Alligator in KDEReview
On Sonntag, 1. November 2020 12:29:43 CET Albert Astals Cid wrote:
> El dijous, 29 d’octubre de 2020, a les 0:34:30 CET, Tobias Fella va
escriure:
> > Since I'm still a beginner at programming with Qt/cpp/qml, I'd
> > appreciate comments or suggestions on my code ;)
>
> You're missing KLocalizedString::setApplicationDomain in your main.cpp
>
> You're not deleteing the Author you create on Entry::Entry, i'm guessing a
> qDeleteAll(m_authors); in the destructor is what you want.
>
> Similarly you seem to also need a qDeleteAll(m_entries) in EntriesModel
> destructor
I strongly suggest not to use raw pointers in C++ code unless you really need
to (e.g. if you create millions of objects and cannot afford the performance
or memory cost). In all other cases you should delegate the object life-time
to C++.
Either use one of the STL smart pointers [1], or, if you insist on Qt, one of
the Qt "equivalents" like QSharedPointer or QScopedPointer.
In the case of Author it would probably be much better to make Author a value-
class. After all, Author is basically just a
struct {
QString name;
QString email;
QString url;
}
Or does it need to be a heavy QObject because of QML? If yes, then you could
also make use of Qt's parent-child-life-cycle-management (see e.g. [2]) and
delegate the deletion of the Author objects to Entry by passing `this` instead
of `nullptr` as parent argument to Author's constructor, i.e. by doing
```
m_authors += new Author(..., this);
```
instead of
```
m_authors += new Author(..., nullptr);
```
By the way, explicitly passing `nullptr` for parent is pretty useless because
`nullptr` is the default value for parent according to the definition of
Author's constructor.
Regards,
Ingo
[1] https://en.cppreference.com/w/cpp/memory
[2] https://doc.qt.io/qt-5/objecttrees.html
signature.asc
Description: This is a digitally signed message part.
