Review Request 117395: Allow compiling dolphin with KF5

Mon Apr 7 22:13:12 BST 2014


> On April 7, 2014, 6:01 p.m., Frank Reininghaus wrote:
> > Thanks for working on this, Alexander!
> > 
> > @Emmanuel: lib/konq seems to be ported already (probably Dolphin would not even compile if it wasn't). KUniqueApplication would not be a problem because kde4support still provides it - merging this into the frameworks branch at some point would not be a problem, and porting away from kde4support could be done at some later point.
> > 
> > I've only had a rather superficial look at the patch so far. It looks good, but it might need some more investigation and testing (see below). In any case, we should discuss two things before this commit is pushed:
> > 
> > 
> > 1. What will happen in which branch in the next months.
> > 
> > I think that we cannot really do major changes both in master and frameworks at the same time. Merging would become a mess. I think that we have two options:
> > 
> > (a) Work mostly in master, merge master into frameworks periodically. Bugs that apply to frameworks only are fixed in frameworks, but no further changes are pushed to frameworks.
> > 
> > (b) Switch master to bugfix-only mode (like in kdelibs), and do all further development in frameworks.
> > 
> > I'm inclined to favor (a) because a KDE SC 4.14 release is planned, and there might be many users who will be using this release for a very long time because they cannot switch to KF5 for some reason. It would be good if we could fix a few issues before we stop working on Dolphin 4.x, like, e.g., the tinting of icons and previews, the lack of a dialog that offers some options when clicking scripts (fixing that probably requires some input from the usability team), and possibly some more rough edges which have been annoying people for some time.
> > 
> > 
> > 2. I understand that you have built Dolphin successfully on top of Qt5/KF5 with your patch, but that it does not work at all (you said that the view is empty). IMHO, this must be fixed before the commit is pushed. Many users got used to applications offering usable Qt5/KF5 ports in their frameworks branches, and some distros will probably start shipping KF5 packages, including those apps, to adventurous users soon. I think that providing a non-functional Dolphin, even in the frameworks branch, is not acceptable. Everyone who tries it will be annoyed, we will get flooded with angry bug reports, and everyone will be frustrated.
> > 
> > 
> > Finally, to everyone who reads this: if you are planning any major changes in Dolphin, please start a discussion on the mailing list BEFORE you start working on it.
> 
> Alexander Richardson wrote:
>     I also think that (a) is the better solution.
>     
>     The problem with the empty view is afaik that classes such as KDirLister have changed their signals from KUrl parameters to QUrl parameters and therefore the signal/slot connection fails. Due to this no update on the view is performed.
>     
>     I will check whether this is the case and then update the review request accordingly.
>

> therefore the signal/slot connection fails
I think we should use the new signal slot syntax in future (compile time check) and this
is a really good place to start with. Maybe you can port each signal 
slot connection to the new syntax, when you fix the KUrl -> QUrl issues?


- Emmanuel


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


On April 6, 2014, 3:29 a.m., Alexander Richardson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117395/
> -----------------------------------------------------------
> 
> (Updated April 6, 2014, 3:29 a.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Allow compiling Dolphin with KF5
> 
> This does not work properly yet, there are probably quite a few bad signal/
> slot connections due to KUrl -> QUrl. But at least dolphin starts without
> crashing.
> 
> Accessibility is not ported since that changed quite a lot from Qt4 -> Qt5
> and I have no idea how it is supposed to be used.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt d01c9b6 
>   dolphin/CMakeLists.txt 4b43d67 
>   dolphin/src/CMakeLists.txt 3f58479 
>   dolphin/src/dolphinapplication.cpp 8e83a85 
>   dolphin/src/dolphincontextmenu.cpp f295de7 
>   dolphin/src/dolphinmainwindow.cpp 8473014 
>   dolphin/src/dolphinpart.h 7146b46 
>   dolphin/src/dolphinpart.cpp 9081731 
>   dolphin/src/dolphinpart_ext.h c05962c 
>   dolphin/src/dolphinviewcontainer.h 31612f1 
>   dolphin/src/dolphinviewcontainer.cpp 768fd5e 
>   dolphin/src/kitemviews/kfileitemlistview.cpp fd01f2c 
>   dolphin/src/kitemviews/kfileitemmodel.cpp fd773e1 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp 0865d40 
>   dolphin/src/kitemviews/kitemlistview.cpp 82f8a20 
>   dolphin/src/kitemviews/private/kfileitemmodeldirlister.h 688ee9c 
>   dolphin/src/kitemviews/private/kfileitemmodelsortalgorithm.h 1d56894 
>   dolphin/src/main.cpp a8e785a 
>   dolphin/src/panels/folders/treeviewcontextmenu.h 0b3fd79 
>   dolphin/src/panels/folders/treeviewcontextmenu.cpp fa8844d 
>   dolphin/src/panels/information/informationpanel.cpp eda74f3 
>   dolphin/src/panels/information/informationpanelcontent.cpp b2dd158 
>   dolphin/src/panels/information/phononwidget.cpp a36ada1 
>   dolphin/src/panels/places/placesitemeditdialog.cpp 08c910d 
>   dolphin/src/panels/places/placesitemmodel.cpp baa770d 
>   dolphin/src/panels/places/placespanel.cpp d5308ea 
>   dolphin/src/panels/terminal/terminalpanel.h 374476e 
>   dolphin/src/panels/terminal/terminalpanel.cpp bfd3002 
>   dolphin/src/search/dolphinsearchbox.cpp c178c43 
>   dolphin/src/search/filenamesearchprotocol.cpp 4d6f59f 
>   dolphin/src/settings/dolphinsettingsdialog.cpp 609e2ab 
>   dolphin/src/settings/general/behaviorsettingspage.cpp cbbde1d 
>   dolphin/src/settings/general/configurepreviewplugindialog.cpp 3ca08df 
>   dolphin/src/settings/general/previewssettingspage.cpp 38b61b9 
>   dolphin/src/settings/kcm/kcmdolphingeneral.cpp 26cb580 
>   dolphin/src/settings/kcm/kcmdolphinnavigation.cpp 36345a5 
>   dolphin/src/settings/kcm/kcmdolphinservices.cpp 6d8c761 
>   dolphin/src/settings/kcm/kcmdolphinviewmodes.cpp a7a9db3 
>   dolphin/src/settings/serviceitemdelegate.h ea9681a 
>   dolphin/src/settings/serviceitemdelegate.cpp 7538e03 
>   dolphin/src/settings/services/servicessettingspage.cpp 48e816b 
>   dolphin/src/settings/viewmodes/viewsettingspage.cpp 4f8a3f0 
>   dolphin/src/settings/viewpropertiesdialog.cpp 574f8e1 
>   dolphin/src/views/dolphinitemlistview.cpp 4799d76 
>   dolphin/src/views/dolphinremoteencoding.cpp 04b350e 
>   dolphin/src/views/dolphinview.h 3731464 
>   dolphin/src/views/dolphinview.cpp 9f5f48a 
>   dolphin/src/views/dolphinviewactionhandler.cpp 48ec95c 
>   dolphin/src/views/renamedialog.cpp d8dbd77 
>   dolphin/src/views/tooltips/filemetadatatooltip.cpp b726996 
>   dolphin/src/views/tooltips/tooltipmanager.cpp bd69483 
>   dolphin/src/views/versioncontrol/updateitemstatesthread.cpp 6457f60 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.h d12d2cf 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.cpp 36345d9 
>   dolphin/src/views/viewproperties.cpp d4ecfaf 
> 
> Diff: https://git.reviewboard.kde.org/r/117395/diff/
> 
> 
> Testing
> -------
> 
> Compiles, starts (but directory view is empty)
> 
> 
> Thanks,
> 
> Alexander Richardson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140407/0ed553ba/attachment.htm>