> 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 > > Dawit Alemayehu wrote: > Nope. I agree with that as well.
This review has been submitted with commit e45d4310d6ce64634e7ed7a24f2604bbefcb0bc6 to branch master. - 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 > >
