[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-frameworks-devel
Subject: Re: Review Request 119895: New class for 5.2 to help user to migrate config files and ui file to new
From: "David Faure" <faure () kde ! org>
Date: 2014-08-22 13:26:54
Message-ID: 20140822132654.8734.66267 () probe ! kde ! org
[Download RAW message or body]
--===============4306407744310519820==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
On Aug. 22, 2014, 9:39 a.m., Laurent Montel wrote:
> > Just a general question: do we really want a porting class in core addons?
>
> Laurent Montel wrote:
> Kdelibs4Migration is already in this addons.
> Where do you want to put it ? In which module ?
>
> Kevin Krammer wrote:
> I just found it strange.
> To me it is like having Qt3Support in QtCore. But I don't know what the goal of KDE \
> core addons is, so providing compatibility stuff might very well be part of its \
> scope.
> David Faure wrote:
> Well, it's not the same. You want to be able to port away from Qt3support / \
> kdelibs4support at some point, to stop linking to it.
> On the other hand, you need to keep calling kdelibs4migrator for the entire 5.x \
> lifetime, since you can't know at which point the last users will switch. So you \
> don't want such code in a compatibility library that you're trying to stop linking \
> to.
> Kevin Krammer wrote:
> Well, that is only the case for applications which used to use kdelibs in their Qt4 \
> version. It is just a single class for now, but if we also want to support data \
> migration at some point this is going to need asynchronous copying, etc. adding to \
> the complexity of a library targetted at more than just ported KDE applications, \
> no?
> Laurent Montel wrote:
> For data, not all applications needs it. So I don't know if I will add in this \
> class. But indeed if we need to put it in this class we need to make it async.
> But what is the problem with it ?
>
> As David wrote we need to have it in all kf5 life so we need to put it in a \
> specific module and we can't put it in kdelibs4support module.
> Kevin Krammer wrote:
> I have to say I am not really sure what the goal here is.
> The base need is a way for applicaiton developers to find their old files, \
> basically a KStandardDirs implementation. If we really want to provide tools on top \
> of that, wouldn't a dedicated framework be a better choice? How likely does an \
> application only have config and ui.rc and no data?
I think Laurent is thinking of kdepim, where the data (akonadi DB etc.) was already \
in XDG dirs anyway, so it doesn't need to be migrated.
Many other apps don't have local data at all (okular, gwenview, kolourpaint, etc. \
etc.). At most a config file and ui.rc files, which is now covered.
Apps with specific data would have to handle that specifically anyway.
So we're left with two classes (one returning paths and one migrating the common case \
of foorc and fooui.rc), which "pollute" kcoreaddons to avoid creating a whole \
framework just for two classes. I can't exactly see how this would be a problem for \
other kcoreaddons users though. They're not using all of the QtCore classes either, \
for sure :-)
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119895/#review65016
-----------------------------------------------------------
On Aug. 22, 2014, 10:55 a.m., Laurent Montel wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119895/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2014, 10:55 a.m.)
>
>
> Review request for KDE Frameworks, David Faure and Kevin Krammer.
>
>
> Repository: kcoreaddons
>
>
> Description
> -------
>
> Class helps user to migrate config file and ui file to new location
>
>
> Diffs
> -----
>
> autotests/CMakeLists.txt 75d1293
> autotests/data/appui1rc PRE-CREATION
> autotests/data/appuirc PRE-CREATION
> autotests/data/foo1rc PRE-CREATION
> autotests/data/foorc PRE-CREATION
> autotests/kdelibs4configmigratortest.cpp PRE-CREATION
> src/lib/CMakeLists.txt 26eb5a1
> src/lib/util/kdelibs4configmigrator.h PRE-CREATION
> src/lib/util/kdelibs4configmigrator.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/119895/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Laurent Montel
>
>
--===============4306407744310519820==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119895/">https://git.reviewboard.kde.org/r/119895/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On August 22nd, 2014, 9:39 a.m. UTC, <b>Kevin \
Krammer</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Just a general question: do we really want a porting \
class in core addons?</p></pre> </blockquote>
<p>On August 22nd, 2014, 9:57 a.m. UTC, <b>Laurent Montel</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Kdelibs4Migration is already in this addons.<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> \
Where do you want to put it ? In which module ?</p></pre> </blockquote>
<p>On August 22nd, 2014, 10:41 a.m. UTC, <b>Kevin Krammer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
just found it strange.<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> To me it is like having Qt3Support in \
QtCore. But I don't know what the goal of KDE core addons is, so providing \
compatibility stuff might very well be part of its scope.</p></pre> </blockquote>
<p>On August 22nd, 2014, 12:06 p.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well, \
it's not the same. You want to be able to port away from Qt3support / kdelibs4support \
at some point, to stop linking to it.</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">On the other hand, you \
need to keep calling kdelibs4migrator for the entire 5.x lifetime, since you can't \
know at which point the last users will switch. So you don't want such code in a \
compatibility library that you're trying to stop linking to.</p></pre> </blockquote>
<p>On August 22nd, 2014, 12:19 p.m. UTC, <b>Kevin Krammer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well, \
that is only the case for applications which used to use kdelibs in their Qt4 \
version.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> It is just a single class for now, but if we also \
want to support data migration at some point this is going to need asynchronous \
copying, etc. adding to the complexity of a library targetted at more than just \
ported KDE applications, no?</p></pre> </blockquote>
<p>On August 22nd, 2014, 12:36 p.m. UTC, <b>Laurent Montel</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">For \
data, not all applications needs it. So I don't know if I will add in this class.<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" /> But indeed if we need to put it in this class we need to make it \
async.<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> But what is the problem with it ?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">As David wrote we need to have it in all kf5 life so \
we need to put it in a specific module and we can't put it in kdelibs4support \
module.</p></pre> </blockquote>
<p>On August 22nd, 2014, 1:09 p.m. UTC, <b>Kevin Krammer</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
have to say I am not really sure what the goal here is.<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> The \
base need is a way for applicaiton developers to find their old files, basically a \
KStandardDirs implementation.<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> If we really want to provide tools on \
top of that, wouldn't a dedicated framework be a better choice?<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;" /> How \
likely does an application only have config and ui.rc and no data?</p></pre> \
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I \
think Laurent is thinking of kdepim, where the data (akonadi DB etc.) was already in \
XDG dirs anyway, so it doesn't need to be migrated.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Many \
other apps don't have local data at all (okular, gwenview, kolourpaint, etc. etc.). \
At most a config file and ui.rc files, which is now covered.</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Apps \
with specific data would have to handle that specifically anyway.</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">So we're left with two classes (one returning paths and one migrating the \
common case of foorc and fooui.rc), which "pollute" kcoreaddons to avoid creating a \
whole framework just for two classes. I can't exactly see how this would be a problem \
for other kcoreaddons users though.<br style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;" /> They're not using all \
of the QtCore classes either, for sure :-)</p></pre> <br />
<p>- David</p>
<br />
<p>On August 22nd, 2014, 10:55 a.m. UTC, Laurent Montel wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for KDE Frameworks, David Faure and Kevin Krammer.</div>
<div>By Laurent Montel.</div>
<p style="color: grey;"><i>Updated Aug. 22, 2014, 10:55 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kcoreaddons
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Class helps user to migrate config file and ui file to \
new location</p></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>autotests/CMakeLists.txt <span style="color: grey">(75d1293)</span></li>
<li>autotests/data/appui1rc <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/data/appuirc <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/data/foo1rc <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/data/foorc <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/kdelibs4configmigratortest.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/lib/CMakeLists.txt <span style="color: grey">(26eb5a1)</span></li>
<li>src/lib/util/kdelibs4configmigrator.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>src/lib/util/kdelibs4configmigrator.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119895/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============4306407744310519820==--
_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic