Review Request 118314: KUrl -> QUrl

Frank Reininghaus frank78ac at googlemail.com
Fri May 30 20:13:07 BST 2014 


> On May 26, 2014, 4:02 p.m., Frank Reininghaus wrote:
> > Thanks for the patch. I think that we all agree that we should port away from kdelibs4support at some point, but is there a reason for doing it right now (i.e., does it fix a bug or cause some other noticeable improvement)?
> > 
> > I'm asking because every large-scale change in the frameworks branch makes merging master into frameworks more painful. Merging is already non-trivial in some cases, due to the adjustments that were required to build with Qt5/KF5, and the replacement of all connect statements with the new syntax (but this at least fixed a few porting issues that would have been hard to spot otherwise, and which had to be fixed to make Dolphin work at all).
> > 
> > This is just the reason why I proposed in https://git.reviewboard.kde.org/r/117395/ that only fixes for frameworks-specific bugs are done in frameworks branch for the time being.
> 
> Emmanuel Pescosta wrote:
>     > only fixes for frameworks-specific bugs are done in frameworks branch for the time being.
>     Oh sorry, I missed that :(
>     Makes sense.
>     
>     Ok, I'll leave this review request open and when we start with the KF5 port I'll update this patch. Ok?
> 
> Frank Reininghaus wrote:
>     Yes, updating it when we stop merging stuff from 4.x to frameworks makes sense! I hope that most of the KUrl -> QUrl transition was done in an automated way anyway, such that it will not be too much work to update it in the future.
> 
> Aleix Pol Gonzalez wrote:
>     It's not automatic, the pain you're saving by not merging the patch you'll suffer it by trying to rebase it back.
> 
> Frank Reininghaus wrote:
>     OK, so the argument is "the KUrl porting has been done already, so we have to suffer the merging pain, in order to prevent that the KUrl -> QUrl porting has to be done again"? If that is what most people think, feel free to push the patch once all issues are resolved, I don't want to be the one who forces people to throw away useful work.
>     
>     I really, really hope that people will start discussing any major changes BEFORE they start working on it at some point, but considering that I repeat this over and over again, and nobody cares, probably this hope is in vain :-(
> 
> Aleix Pol Gonzalez wrote:
>     Coming up with a roadmap could be useful.

To summarize things that I said earlier on the mailing list and in review requests, I think that we should not do any unnecessary changes in frameworks (i.e., changes which do not fix a frameworks-specific bug) as long as we do Dolphin 4.x releases. The reasoning is that master should be mergeable into frameworks easily to ensure that all bug fixes reach the frameworks branch - this is much easier than forward-porting each bug fix individually IMHO.

Concerning the question how long we do Dolphin 4.x releases: to my knowledge, it has not been decided yet if KDE SC 4.14 will be the last 4.x release or not, but if there will be a KDE SC 4.15, I currently see no good reason why Dolphin should not be part of it.

Now this is all my personal opinion, and if others can provide good arguments why we should do it in a different way, we can discuss it, of course. However, nobody has raised any objections so far as far as I remember. Therefore, the current roadmap for the frameworks branch is "no changes which do not fix a frameworks-specific bug or improve the user experience on frameworks in another way" from my point of view. Again, if anyone has other views, please speak up.

Note that this does not mean that we should neglect the frameworks branch. We should keep it as usable and as bug-free as possible, I test it myself from time to time, and I even tried to fix a frameworks-specific issue ( https://git.reviewboard.kde.org/r/118269/ ) which currently makes Dolphin on frameworks rather unreleasable. I also greatly appreciate that Alexander has taken the effort to port Dolphin to Qt5/KF5 because it's a good test for the refactored frameworks libraries, and may help to find and fix more problems that were introduced in the kdelibs splitting process.


- Frank


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


On May 25, 2014, 1:38 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118314/
> -----------------------------------------------------------
> 
> (Updated May 25, 2014, 1:38 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Ported Dolphin from KUrl to QUrl.
> 
> Moved the "new name checking" from DolphinView::slotRoleEditingFinished to KFileItemModel::setData,
> because we need the same url modifying code for both functions - avoid code duplication + porting mistakes.
> 
> RenameDialog:
> Changed renameItems to renameItem, we do the url modifying there for all two cases -
> avoid code duplication + porting mistakes.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinapplication.cpp 8144da8 
>   dolphin/src/dolphincontextmenu.h 180f917 
>   dolphin/src/dolphincontextmenu.cpp 51351f0 
>   dolphin/src/dolphinmainwindow.h 1192f6e 
>   dolphin/src/dolphinmainwindow.cpp 049c440 
>   dolphin/src/dolphinpart.h 9ab1e80 
>   dolphin/src/dolphinpart.cpp 479b809 
>   dolphin/src/dolphinpart_ext.h 5272df6 
>   dolphin/src/dolphinpart_ext.cpp fb7a4d2 
>   dolphin/src/dolphinviewcontainer.h a7a9969 
>   dolphin/src/dolphinviewcontainer.cpp 7610625 
>   dolphin/src/kitemviews/kfileitemmodel.h 62a283d 
>   dolphin/src/kitemviews/kfileitemmodel.cpp 51bf546 
>   dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp acb3e0f 
>   dolphin/src/kitemviews/kstandarditemlistwidget.cpp 7a9f31a 
>   dolphin/src/kitemviews/private/kdirectorycontentscounter.cpp cd448e2 
>   dolphin/src/kitemviews/private/kfileitemclipboard.h 86eb8e9 
>   dolphin/src/kitemviews/private/kfileitemclipboard.cpp ebf50e2 
>   dolphin/src/kitemviews/private/kfileitemmodeldirlister.h da01d20 
>   dolphin/src/panels/folders/folderspanel.h 14d8e87 
>   dolphin/src/panels/folders/folderspanel.cpp 0b2ea38 
>   dolphin/src/panels/folders/treeviewcontextmenu.cpp 2e59ae8 
>   dolphin/src/panels/information/informationpanel.h c68b66e 
>   dolphin/src/panels/information/informationpanel.cpp 4ad1276 
>   dolphin/src/panels/information/informationpanelcontent.h 67fdf6c 
>   dolphin/src/panels/information/informationpanelcontent.cpp 9dc59db 
>   dolphin/src/panels/information/phononwidget.h b5aedfe 
>   dolphin/src/panels/information/phononwidget.cpp 4b2cc28 
>   dolphin/src/panels/panel.h a0b25d6 
>   dolphin/src/panels/panel.cpp 14b7c02 
>   dolphin/src/panels/places/placesitem.h 297f12d 
>   dolphin/src/panels/places/placesitem.cpp 1729bbd 
>   dolphin/src/panels/places/placesitemeditdialog.h bf34847 
>   dolphin/src/panels/places/placesitemeditdialog.cpp 0a66ef7 
>   dolphin/src/panels/places/placesitemmodel.h 4a374e5 
>   dolphin/src/panels/places/placesitemmodel.cpp 6ba91c5 
>   dolphin/src/panels/places/placespanel.h 16112e8 
>   dolphin/src/panels/places/placespanel.cpp d3614c9 
>   dolphin/src/panels/terminal/terminalpanel.h 987ee47 
>   dolphin/src/panels/terminal/terminalpanel.cpp 5c72652 
>   dolphin/src/search/dolphinsearchbox.h 53b12ff 
>   dolphin/src/search/dolphinsearchbox.cpp df96f74 
>   dolphin/src/search/filenamesearchprotocol.h f691f99 
>   dolphin/src/search/filenamesearchprotocol.cpp b56a995 
>   dolphin/src/settings/applyviewpropsjob.h 68fdcc4 
>   dolphin/src/settings/applyviewpropsjob.cpp 9849216 
>   dolphin/src/settings/dolphinsettingsdialog.h 2de1950 
>   dolphin/src/settings/dolphinsettingsdialog.cpp 8df2996 
>   dolphin/src/settings/general/behaviorsettingspage.h 7a9c2f0 
>   dolphin/src/settings/general/behaviorsettingspage.cpp 7633b82 
>   dolphin/src/settings/general/generalsettingspage.h 0d28664 
>   dolphin/src/settings/general/generalsettingspage.cpp 8a8adf8 
>   dolphin/src/settings/startup/startupsettingspage.h 29cdc63 
>   dolphin/src/settings/startup/startupsettingspage.cpp 7f8c2d5 
>   dolphin/src/settings/viewpropertiesdialog.cpp 593d1c4 
>   dolphin/src/settings/viewpropsprogressinfo.h 6f8c763 
>   dolphin/src/settings/viewpropsprogressinfo.cpp a4a45da 
>   dolphin/src/statusbar/dolphinstatusbar.h 4d6dbb2 
>   dolphin/src/statusbar/dolphinstatusbar.cpp 9f17c8e 
>   dolphin/src/statusbar/spaceinfoobserver.h d2fb6eb 
>   dolphin/src/statusbar/spaceinfoobserver.cpp 9125a93 
>   dolphin/src/statusbar/statusbarspaceinfo.h 1065d9f 
>   dolphin/src/statusbar/statusbarspaceinfo.cpp 3692947 
>   dolphin/src/tests/kfileitemmodelbenchmark.cpp 66918b6 
>   dolphin/src/tests/kfileitemmodeltest.cpp 48e72e8 
>   dolphin/src/tests/testdir.h 0d3c5dd 
>   dolphin/src/tests/testdir.cpp 8938e60 
>   dolphin/src/views/dolphinnewfilemenuobserver.h 3b5014a 
>   dolphin/src/views/dolphinremoteencoding.h b317cc1 
>   dolphin/src/views/dolphinremoteencoding.cpp bf00e33 
>   dolphin/src/views/dolphinview.h 4ba1029 
>   dolphin/src/views/dolphinview.cpp 0e43dcd 
>   dolphin/src/views/dolphinviewactionhandler.cpp 3955f25 
>   dolphin/src/views/draganddrophelper.h eda5fc5 
>   dolphin/src/views/draganddrophelper.cpp f8ae0ad 
>   dolphin/src/views/renamedialog.h 29ef8bd 
>   dolphin/src/views/renamedialog.cpp 3b94e01 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.h 2c07b06 
>   dolphin/src/views/versioncontrol/versioncontrolobserver.cpp 9033e19 
>   dolphin/src/views/viewmodecontroller.h f476595 
>   dolphin/src/views/viewmodecontroller.cpp 26e1818 
>   dolphin/src/views/viewproperties.h 69b507f 
>   dolphin/src/views/viewproperties.cpp bcea062 
> 
> Diff: https://git.reviewboard.kde.org/r/118314/diff/
> 
> 
> Testing
> -------
> 
> KFileItemModelTest fails. (Expanded items)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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

More information about the kfm-devel mailing list