Review Request: Remove usage of KIO::SlaveConfig from Dolphin

Dawit Alemayehu adawit at kde.org
Sun Nov 25 18:00:38 GMT 2012



> On Nov. 22, 2012, 7:45 p.m., David Faure wrote:
> > This probably needs a #ifdef on the kdelibs version number, then, since apparently kde-baseapps master only requires kdelibs-4.8...
> > 
> > It says find_package(KDE4 4.8.0 REQUIRED). Not sure why not 4.9 at least, but that wouldn't help anyway.
> >
> 
> Albert Astals Cid wrote:
>     Because when i sent an email to k-c-d with "Dependency Freeze for 4.10 in two weeks" asking for depencies like that to be increased noone suggested changing it
>
> 
> Frank Reininghaus wrote:
>     Thanks Dawit for implementing this!
>     
>     About the kdelibs version issue: wouldn't the easiest solution be to raise the version requirement once master is open again and push the patch to master then? It won't hurt much if the KIO::SlaveConfig call stays in the 4.10 branch, right?
> 
> David Faure wrote:
>     Sounds like a plan.
> 
> Dawit Alemayehu wrote:
>     For the record I did raise my hand in response to your k-c-d post Albert. kde-baseapps in the 4.10 branch REQUIRES a minimum of kdelibs v4.9.80 to work correctly. Otherwise, cookie handling will be completely screwed up if the user launches the cookie configuration dialog. Based on the conversion in the thread you created, I personally did change the KDE_MIN_VERSION flag to 4.9.80, but I did not do anything to the KDE4 flag because I did not know I was supposed to update that too. :( It still would be safe to update KDE4 to the correct version since people won't be able to compile kde-baseapps against any kdelibs version older than v4.9.80 right now, no ?
>     
>     Anyhow, there is no reason to rush this into the 4.10 branch ; so waiting for master  to open and the pushing it there indeed sounds like a plan.
> 
> Albert Astals Cid wrote:
>     Right, you did, i thougth that was a different module, sorry about that.
>     
>     Been reading FindKDE4Internal.cmake and it seems that what you did is correct (even if deprecated) or so says the documentation:
>     
>     #  This module allows to depend on a particular minimum version of kdelibs.
>     #  To acomplish that one should use the appropriate cmake syntax for
>     #  find_package. For example to depend on kdelibs >= 4.1.0 one should use
>     #
>     #  find_package(KDE4 4.1.0 REQUIRED)
>     #
>     #  In earlier versions of KDE you could use the variable KDE_MIN_VERSION to
>     #  have such a dependency. This variable is deprecated with KDE 4.2.0, but
>     #  will still work to make the module backwards-compatible.
>     
>     The change you are suggesting in this thread would work with 4.9.80 (beta 1) as released or would need something newer?
> 
> Dawit Alemayehu wrote:
>     It would need something newer because I added the new function to KProtocolManager after the tagging for 4.9.80 (beta 1).
> 
> Albert Astals Cid wrote:
>     *Personally* I would have no problem in increasing 4.9.80 to 4.9.90 since for a regular user/developer 4.9.80 and 4.9.90 are as "bad" as the other, but OTOH what Frank/David say makes a lot of sense and this does not really seem like something that would need lots of maintainance and thus having two "different" codebases might be a problem, so I'd vote for putting it only in master once we reopen unless you really have a strong opinion against that

Nope. I agree with that as well.


- Dawit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107421/#review22391
-----------------------------------------------------------


On Nov. 22, 2012, 5:26 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107421/
> -----------------------------------------------------------
> 
> (Updated Nov. 22, 2012, 5:26 p.m.)
> 
> 
> Review request for KDE Base Apps, David Faure and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> The attached patch ports Dolphin away from using KIO::SlaveConfig::configData into the newly added KProtocolManager::charsetFor API. KIO::SlaveConfig was never intended to be a public class and as such it is going to go away in KF5.
> 
> Please note that you need to latest kdelibs from the 4.10 branch in order to compile this change.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/dolphinremoteencoding.cpp 375b3fd 
> 
> Diff: http://git.reviewboard.kde.org/r/107421/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121125/d1f7c25c/attachment.htm>


More information about the kde-core-devel mailing list