RKWard | Resume KDE review process (#2)

[Thomas Friedrichsmeier]

Thomas Friedrichsmeier commented:


Summary of critical comments raised and their status:

* https://marc.info/?l=kde-core-devel&m=153912336114603&w=2 (aacid):
  * the i18n folder seems like from the past and something you should not need if in kde \
infrastructure. Could you delete it?
  * Also I'd suggest you compile enabling -DECM_ENABLE_SANITIZERS="address;undefined"
    * Thanks for the hint, did not know that switch. One effect of this is a lot of false positive "runtime errors", when casting a half-destroyed QObject* to its original (derived) type, in order to remove it from containers (e.g. a QMap of KActionCollection()s). Is there an elegant way around this?
  * There's a few memory leaks (reported at exit) that you may want to have a look.
    * Most (but not all) are fixed, now.
  * And there's also a few undefined behaviour warnings on exit, you've them marked as \
"known" things but it'd be good if you could find a way to fix them.
  * Your help menu is for some reason missing the Change Language option, i tried to do \
it a quick fix but could not, i would appreciate if you could find a way to only \
define the extra actions and not all of them (like we do for example in okular).

* https://marc.info/?l=kde-core-devel&m=153916691226377&w=2 (jriddel)
  * It installs two desktop files which creates duplicate menu entries /usr/share/applications/org.kde.rkward-open.desktop /usr/share/applications/org.kde.rkward.desktop
    * Completed following suggestions by Thomas Baumgart and Meik: https://marc.info/?l=kde-core-devel&m=153942961310071&w=2
  * The .desktop files call it a "GUI for R" which is not a great description, everything in the menu is a GUI.  I recommend "R Statistical Programming" or "IDE for R" maybe.
    * Suggestion by Meik: Statistics with R (https://marc.info/?l=kde-core-devel&m=153942595709279&w=2)
  * I tidied up the files with the icon licence as they could easily be lost.
    * Done by jriddel
  * It depends on WebKit which is not supported, could this be ported to WebEngine?
    * Comment by Lisandro (https://marc.info/?l=kde-core-devel&m=154218478600505&w=2)
    * Done in the meantime by supporting WebEngine, optionally (and by default)
  * It's uncommon having debian/ packaging directly in the source and there's also debian-official/ which could get confusing and out-of-sync and messy.  I recommend moving them to another archive.
  * Storing the packaging in KDE neon Git would be cool as we already have packaging for all the rest of KDE software.  Using salsa.debian pkg-kde team would also make sense but would need different
permissions.  Either way we should get this into Neon, give me a ping to sort that out.
    * Partially done
  * Prelim. answer to jriddel: https://marc.info/?l=kde-core-devel&m=153918336832295&w=2

* https://marc.info/?l=kde-core-devel&m=154118912915065&w=2 (jriddel)
  * There's no appstream metainfo file nor product-screenshot https://community.kde.org/Guidelines_and_HOWTOs/AppStream
    * Done: Contributed by Meik and Yuri

-- 
Reply to this email directly or view it on GitLab: https://invent.kde.org/education/rkward/-/issues/2#note_56280
You're receiving this email because of your account on invent.kde.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/rkward-tracker/attachments/20200606/f035efee/attachment.htm>