<div dir="ltr"><div>I've created an issue for the kdereview process</div><div><br></div><div><a href="https://invent.kde.org/education/rkward/-/issues/23">https://invent.kde.org/education/rkward/-/issues/23</a></div><div><br></div><div>Jonathan</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, 26 Mar 2022 at 20:34, Thomas Friedrichsmeier <<a href="mailto:thomas.friedrichsmeier@kdemail.net">thomas.friedrichsmeier@kdemail.net</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">Hi!<br>
<br>
KDE.org has been our home for a 7.5(!) years, now (after over a<br>
decade on sourceforge), but we still haven't left playground... After a<br>
lot of procrastination on that matter, a previous review failed due to<br>
lack of time on my part. Sorry! Now, finally, I'd like to ask you to<br>
start reviewing RKWard for inclusion into exragear / education, once<br>
more.<br>
<br>
RKWard is an easy to use and easily extensible IDE/GUI for R (a<br>
powerful language and environment for statistical computing, if you<br>
did not know it, yet). It aims to combine the power of the<br>
R-language with the ease of use of commercial statistics tools.<br>
<br>
RKWard is used productively on Linux/BSD, Mac, and Windows.<br>
<br>
<br>
<br>
Here's a summary of the critical comments we got in the previous round<br>
of review (<a href="https://marc.info/?l=kde-core-devel&m=153883367600690&w=2" rel="noreferrer" target="_blank">https://marc.info/?l=kde-core-devel&m=153883367600690&w=2</a>):<br>
<br>
Albert Astals Cid remarked<br>
(<a href="https://marc.info/?l=kde-core-devel&m=153912336114603&w=2" rel="noreferrer" target="_blank">https://marc.info/?l=kde-core-devel&m=153912336114603&w=2</a>):<br>
<br>
> the i18n folder seems like from the past and something you should not<br>
> need if in kde infrastructure. Could you delete it?<br>
<br>
We cannot easily get rid of this, entirely, as we are shipping<br>
(non-compiled) plugins that keep their message catalogs in relative<br>
paths, and thus we have to install .mo files to custom locations. Pino<br>
Toscano has helped to bring this more in line with standard KDE.org<br>
processes (<a href="https://invent.kde.org/education/rkward/-/merge_requests/2" rel="noreferrer" target="_blank">https://invent.kde.org/education/rkward/-/merge_requests/2</a>).<br>
<br>
> Also I'd suggest you compile enabling<br>
> -DECM_ENABLE_SANITIZERS="address;undefined"<br>
<br>
Thanks for the hint, did not know that switch. One effect of this is a<br>
lot of false positive "runtime errors", when casting a half-destroyed<br>
QObject* to its original (derived) type, in order to remove it from<br>
containers (e.g. a QHash of KActionCollection()s). Is there an elegant<br>
way around this?<br>
<br>
> There's a few memory leaks (reported at exit) that you may want to<br>
> have a look.<br>
<br>
I fixed a lot of that, but didn't work out all of them.<br>
<br>
> And there's also a few undefined behaviour warnings on exit, you've<br>
> them marked as "known" things but it'd be good if you could find a way<br>
> to fix them.<br>
<br>
Not quite sure what you were referring to, esp. what I may have marked<br>
as known behavior. I did fix several quirks at exit since this comment<br>
(and I keep introducing new ones, all the time).<br>
<br>
> Your help menu is for some reason missing the Change Language option,<br>
> i tried to do it a quick fix but could not, i would appreciate if you<br>
> could find a way to only define the extra actions and not all of them<br>
> (like we do for example in okular).<br>
<br>
I've added the switch application language option (in Settings rather<br>
than Help). Our help menu is perhaps more customized than meets the eye<br>
on first glance. For instance, we have an app-integrated help system<br>
instead of external handbook (at least as the primary documentation),<br>
and our bug report dialog is beefed up to include a lot of extra<br>
information by default (mostly importantly: version of R). I guess it<br>
would be possible to hack KHelpMenu this way, but at least at some<br>
point in the past it seemed cleaner to implement "from scratch" (but<br>
using the default actions, where applicable).<br>
<br>
<br>
<br>
Comments by Jonathan Riddel<br>
(<a href="https://marc.info/?l=kde-core-devel&m=153916691226377&w=2" rel="noreferrer" target="_blank">https://marc.info/?l=kde-core-devel&m=153916691226377&w=2</a>):<br>
<br>
> It installs two desktop files which creates duplicate menu entries<br>
> /usr/share/applications/org.kde.rkward-open.desktop<br>
> /usr/share/applications/org.kde.rkward.desktop<br>
<br>
Completed following suggestions by Thomas Baumgart and Meik Michalke:<br>
<a href="https://marc.info/?l=kde-core-devel&m=153942961310071&w=2" rel="noreferrer" target="_blank">https://marc.info/?l=kde-core-devel&m=153942961310071&w=2</a><br>
<br>
> The .desktop files call it a "GUI for R" which is not a great<br>
> description, everything in the menu is a GUI. I recommend "R<br>
> Statistical Programming" or "IDE for R" maybe.<br>
<br>
Renamed to Statistics with R, following Meik's suggestion<br>
(<a href="https://marc.info/?l=kde-core-devel&m=153942595709279&w=2" rel="noreferrer" target="_blank">https://marc.info/?l=kde-core-devel&m=153942595709279&w=2</a>).<br>
<br>
> I tidied up the files with the icon licence as they could easily be<br>
> lost.<br>
<br>
Done by Jonathan.<br>
<br>
> It depends on WebKit which is not supported, could this be ported to<br>
> WebEngine?<br>
<br>
Meanwhile, RKWard can be compiled to use QtWebEngine (and this is the<br>
default), but support for webkit has not been dropped, because MinGW is<br>
an important platform for us.<br>
<br>
> It's uncommon having debian/ packaging directly in the source and<br>
> there's also debian-official/ which could get confusing and<br>
> out-of-sync and messy. I recommend moving them to another archive.<br>
> Storing the packaging in KDE neon Git would be cool as we already<br>
> have packaging for all the rest of KDE software.<br>
<br>
Meanwhile packaging has been taken over by the debian-qt-kde team<br>
(thank you so much!). Jonathan himself added RKWard to neon. We still<br>
have a (basic) debian packaging to support nightly builds in our Ubuntu<br>
PPAs (which fill a somewhat different gap than Neon), but this is now<br>
kept in a separate repository.<br>
<br>
<br>
<br>
Jonathan commented in a separate mail<br>
(<a href="https://marc.info/?l=kde-core-devel&m=154118912915065&w=2" rel="noreferrer" target="_blank">https://marc.info/?l=kde-core-devel&m=154118912915065&w=2</a>)<br>
<br>
> There's no appstream metainfo file nor product-screenshot<br>
> <a href="https://community.kde.org/Guidelines_and_HOWTOs/AppStream" rel="noreferrer" target="_blank">https://community.kde.org/Guidelines_and_HOWTOs/AppStream</a><br>
<br>
Done: Contributed by Meik and Yuri<br>
<br>
<br>
<br>
Thanks to all for your feedback last time (I left out feedback that did<br>
no criticize anything, but I hope I have not left out anything<br>
substantial in my summary)!<br>
<br>
Looking forward to your comments.<br>
<br>
Thomas<br>
</blockquote></div>