Review Request 128236: Better fix for version dialog contents and startup warnings

[Andrea Scarpino]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128236/#review96683
-----------------------------------------------------------




choqok/choqokapplication.cpp (line 32)
<https://git.reviewboard.kde.org/r/128236/#comment65328>

    Shouldn't be needed



choqok/choqokapplication.cpp (line 43)
<https://git.reviewboard.kde.org/r/128236/#comment65326>

    Just set m_mainWindow(0) above and drop this line



choqok/choqokapplication.cpp (line 53)
<https://git.reviewboard.kde.org/r/128236/#comment65327>

    What if m_mainWindow is already set? Please delete the old one before creating the new MainWindow.


- Andrea Scarpino


On June 18, 2016, 11:56 a.m., Ignacio R. Morelle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128236/
> -----------------------------------------------------------
> 
> (Updated June 18, 2016, 11:56 a.m.)
> 
> 
> Review request for Choqok.
> 
> 
> Bugs: 364408
>     http://bugs.kde.org/show_bug.cgi?id=364408
> 
> 
> Repository: choqok
> 
> 
> Description
> -------
> 
> In /r/128220, while my change does fix the About dialog contents being missing, it does so by introducing new non-fatal errors during application startup:
> 
> ```
> % choqok --version
> org.kde.choqok: Choqok  1.5.80
> kf5.kcoreaddons.kaboutdata: Could not initialize the equivalent properties of Q*Application: no instance (yet) existing.
> org.kde.choqok:
> kf5.kcoreaddons.kaboutdata: QCoreApplication::applicationVersion "" is out-of-sync with KAboutData::applicationData().version "1.5.80"
> kf5.kcoreaddons.kaboutdata: QCoreApplication::organizationDomain "kde.org" is out-of-sync with KAboutData::applicationData().organizationDomain "gnufolks.org"
> kf5.kcoreaddons.kaboutdata: QGuiApplication::desktopFileName "" is out-of-sync with KAboutData::applicationData().desktopFileName "org.gnufolks.choqok"
> org.kde.choqok:
> org.kde.choqok:
> org.kde.choqok:
> org.kde.choqok.lib:
> choqok 1.5.80
> org.kde.choqok.lib: Destructing plugin manager without going through the shutdown process!
> ```
> 
> After looking more into how other KDE applications set up their KAboutData structure, I realized that the original QApplication(ChoqokApplication)/KAboutData initialization order was correct. The correct cause of the version info dialog coming up empty seems to be the MainWindow instance being created in the ChoqokApplication constructor before registering the KAboutData.
> 
> So the proposed fix does involve some refactoring to be able to postpone the construction of the MainWindow until the KAboutData is registered from main(). I believe it's safe to postpone it until after all command line processing and config migration is out of the way.
> 
> Additionally (and this is important), the new patch explicitly sets the organization domain to kde.org in the KAboutData to match the original QApplication data set in the ChoqokApplication ctor, as well as the D-Bus info. Previously, KAboutData was defaulting to gnufolks.org as per the homePageAddress parameter to its ctor. The QApplication data is also now made to explicitly match the KAboutData.
> 
> `--version` output with the new patch applied:
> 
> ```
> % choqok --version
> org.kde.choqok: Choqok  1.5.80
> org.kde.choqok:
> choqok 1.5.80
> ```
> 
> (Worth noting that my experience with Qt thus far is mostly from the Qt 4 times, and I'm new to working with the KDE codebase. Also, sometimes Choqok segfaults on exit for reasons unrelated to either patch. I will report a bug for this separately.)
> 
> 
> Diffs
> -----
> 
>   choqok/choqokapplication.h 2e0f2c3 
>   choqok/choqokapplication.cpp 50fc9cd 
>   choqok/main.cpp 40c0e16 
> 
> Diff: https://git.reviewboard.kde.org/r/128236/diff/
> 
> 
> Testing
> -------
> 
> * Built and ran choqok, both without any command line arguments, or passing either --version or --help.
> 
> 
> Thanks,
> 
> Ignacio R. Morelle
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/choqok-devel/attachments/20160618/ce007178/attachment.html>