[prev in list] [next in list] [prev in thread] [next in thread]
List: kfm-devel
Subject: Re: Review Request 117395: Allow compiling dolphin with KF5
From: "Frank Reininghaus" <frank78ac () googlemail ! com>
Date: 2014-04-07 16:01:18
Message-ID: 20140407160118.26935.13152 () probe ! kde ! org
[Download RAW message or body]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117395/#review55182
-----------------------------------------------------------
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.
- Frank Reininghaus
On April 6, 2014, 1: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, 1: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
>
>
[Attachment #3 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px \
#c9c399 solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/117395/">https://git.reviewboard.kde.org/r/117395/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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.</pre> <br />
<p>- Frank Reininghaus</p>
<br />
<p>On April 6th, 2014, 1:29 a.m. UTC, Alexander Richardson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px \
black solid;"> <tr>
<td>
<div>Review request for Dolphin.</div>
<div>By Alexander Richardson.</div>
<p style="color: grey;"><i>Updated April 6, 2014, 1:29 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-baseapps
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description \
</h1> <table width="100%" bgcolor="#ffffff" cellspacing="0" \
cellpadding="10" style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing \
</h1> <table width="100%" bgcolor="#ffffff" cellspacing="0" \
cellpadding="10" style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Compiles, starts (but directory view is empty)</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> \
</h1> <ul style="margin-left: 3em; padding-left: 0;">
<li>CMakeLists.txt <span style="color: grey">(d01c9b6)</span></li>
<li>dolphin/CMakeLists.txt <span style="color: grey">(4b43d67)</span></li>
<li>dolphin/src/CMakeLists.txt <span style="color: \
grey">(3f58479)</span></li>
<li>dolphin/src/dolphinapplication.cpp <span style="color: \
grey">(8e83a85)</span></li>
<li>dolphin/src/dolphincontextmenu.cpp <span style="color: \
grey">(f295de7)</span></li>
<li>dolphin/src/dolphinmainwindow.cpp <span style="color: \
grey">(8473014)</span></li>
<li>dolphin/src/dolphinpart.h <span style="color: \
grey">(7146b46)</span></li>
<li>dolphin/src/dolphinpart.cpp <span style="color: \
grey">(9081731)</span></li>
<li>dolphin/src/dolphinpart_ext.h <span style="color: \
grey">(c05962c)</span></li>
<li>dolphin/src/dolphinviewcontainer.h <span style="color: \
grey">(31612f1)</span></li>
<li>dolphin/src/dolphinviewcontainer.cpp <span style="color: \
grey">(768fd5e)</span></li>
<li>dolphin/src/kitemviews/kfileitemlistview.cpp <span style="color: \
grey">(fd01f2c)</span></li>
<li>dolphin/src/kitemviews/kfileitemmodel.cpp <span style="color: \
grey">(fd773e1)</span></li>
<li>dolphin/src/kitemviews/kfileitemmodelrolesupdater.cpp <span \
style="color: grey">(0865d40)</span></li>
<li>dolphin/src/kitemviews/kitemlistview.cpp <span style="color: \
grey">(82f8a20)</span></li>
<li>dolphin/src/kitemviews/private/kfileitemmodeldirlister.h <span \
style="color: grey">(688ee9c)</span></li>
<li>dolphin/src/kitemviews/private/kfileitemmodelsortalgorithm.h <span \
style="color: grey">(1d56894)</span></li>
<li>dolphin/src/main.cpp <span style="color: grey">(a8e785a)</span></li>
<li>dolphin/src/panels/folders/treeviewcontextmenu.h <span style="color: \
grey">(0b3fd79)</span></li>
<li>dolphin/src/panels/folders/treeviewcontextmenu.cpp <span style="color: \
grey">(fa8844d)</span></li>
<li>dolphin/src/panels/information/informationpanel.cpp <span \
style="color: grey">(eda74f3)</span></li>
<li>dolphin/src/panels/information/informationpanelcontent.cpp <span \
style="color: grey">(b2dd158)</span></li>
<li>dolphin/src/panels/information/phononwidget.cpp <span style="color: \
grey">(a36ada1)</span></li>
<li>dolphin/src/panels/places/placesitemeditdialog.cpp <span style="color: \
grey">(08c910d)</span></li>
<li>dolphin/src/panels/places/placesitemmodel.cpp <span style="color: \
grey">(baa770d)</span></li>
<li>dolphin/src/panels/places/placespanel.cpp <span style="color: \
grey">(d5308ea)</span></li>
<li>dolphin/src/panels/terminal/terminalpanel.h <span style="color: \
grey">(374476e)</span></li>
<li>dolphin/src/panels/terminal/terminalpanel.cpp <span style="color: \
grey">(bfd3002)</span></li>
<li>dolphin/src/search/dolphinsearchbox.cpp <span style="color: \
grey">(c178c43)</span></li>
<li>dolphin/src/search/filenamesearchprotocol.cpp <span style="color: \
grey">(4d6f59f)</span></li>
<li>dolphin/src/settings/dolphinsettingsdialog.cpp <span style="color: \
grey">(609e2ab)</span></li>
<li>dolphin/src/settings/general/behaviorsettingspage.cpp <span \
style="color: grey">(cbbde1d)</span></li>
<li>dolphin/src/settings/general/configurepreviewplugindialog.cpp <span \
style="color: grey">(3ca08df)</span></li>
<li>dolphin/src/settings/general/previewssettingspage.cpp <span \
style="color: grey">(38b61b9)</span></li>
<li>dolphin/src/settings/kcm/kcmdolphingeneral.cpp <span style="color: \
grey">(26cb580)</span></li>
<li>dolphin/src/settings/kcm/kcmdolphinnavigation.cpp <span style="color: \
grey">(36345a5)</span></li>
<li>dolphin/src/settings/kcm/kcmdolphinservices.cpp <span style="color: \
grey">(6d8c761)</span></li>
<li>dolphin/src/settings/kcm/kcmdolphinviewmodes.cpp <span style="color: \
grey">(a7a9db3)</span></li>
<li>dolphin/src/settings/serviceitemdelegate.h <span style="color: \
grey">(ea9681a)</span></li>
<li>dolphin/src/settings/serviceitemdelegate.cpp <span style="color: \
grey">(7538e03)</span></li>
<li>dolphin/src/settings/services/servicessettingspage.cpp <span \
style="color: grey">(48e816b)</span></li>
<li>dolphin/src/settings/viewmodes/viewsettingspage.cpp <span \
style="color: grey">(4f8a3f0)</span></li>
<li>dolphin/src/settings/viewpropertiesdialog.cpp <span style="color: \
grey">(574f8e1)</span></li>
<li>dolphin/src/views/dolphinitemlistview.cpp <span style="color: \
grey">(4799d76)</span></li>
<li>dolphin/src/views/dolphinremoteencoding.cpp <span style="color: \
grey">(04b350e)</span></li>
<li>dolphin/src/views/dolphinview.h <span style="color: \
grey">(3731464)</span></li>
<li>dolphin/src/views/dolphinview.cpp <span style="color: \
grey">(9f5f48a)</span></li>
<li>dolphin/src/views/dolphinviewactionhandler.cpp <span style="color: \
grey">(48ec95c)</span></li>
<li>dolphin/src/views/renamedialog.cpp <span style="color: \
grey">(d8dbd77)</span></li>
<li>dolphin/src/views/tooltips/filemetadatatooltip.cpp <span style="color: \
grey">(b726996)</span></li>
<li>dolphin/src/views/tooltips/tooltipmanager.cpp <span style="color: \
grey">(bd69483)</span></li>
<li>dolphin/src/views/versioncontrol/updateitemstatesthread.cpp <span \
style="color: grey">(6457f60)</span></li>
<li>dolphin/src/views/versioncontrol/versioncontrolobserver.h <span \
style="color: grey">(d12d2cf)</span></li>
<li>dolphin/src/views/versioncontrol/versioncontrolobserver.cpp <span \
style="color: grey">(36345d9)</span></li>
<li>dolphin/src/views/viewproperties.cpp <span style="color: \
grey">(d4ecfaf)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/117395/diff/" \
style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic