Review Request 118314: KUrl -> QUrl
Frank Reininghaus
frank78ac at googlemail.com
Fri May 30 14:46:48 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.
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 :-(
- 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/ffbf858c/attachment.htm>
More information about the kfm-devel
mailing list