Review Request 118970: libkeduvocdocument: Remove KDELibs4Support dependency.

Inge Wallin inge at lysator.liu.se
Fri Jun 27 06:31:39 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118970/#review61067
-----------------------------------------------------------


I didn't find any large problems in the patch itself, just a few small questions (see below).

One thing to consider, though, is if the base for this patch is too old. After a hiatus, new things have been added to libkdeedu in master and some of those are important. Perhaps it is time to rebase the branch on the current master before going further?


keduvocdocument/CMakeLists.txt
<https://git.reviewboard.kde.org/r/118970/#comment42526>

    Why removed in other places and commented out here?



keduvocdocument/keduvocdocument.cpp
<https://git.reviewboard.kde.org/r/118970/#comment42527>

    I don't say this is wrong. I just want to point out that there are some interesting new features in master that this must work together with.  One of those things is file locking which I can imagine could clash with this.



keduvocdocument/tests/converter.cpp
<https://git.reviewboard.kde.org/r/118970/#comment42528>

    Hmm, the file name says .../tests/converter.  Is this a test or a converter?


- Inge Wallin


On June 27, 2014, 4:55 a.m., Jeremy Whiting wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118970/
> -----------------------------------------------------------
> 
> (Updated June 27, 2014, 4:55 a.m.)
> 
> 
> Review request for KDE Edu, KDE Frameworks, Aleix Pol Gonzalez, and Inge Wallin.
> 
> 
> Repository: libkdeedu
> 
> 
> Description
> -------
> 
> Port tests away from KCmdLineArgs to QCommandLineParser.
> Port from KDE_DEPRECATED to KEDUVOCDOCUMENT_DEPRECATED (Maybe we should remove these?)
> Port from KLocale to klocalizedstring.h
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 4feb10b67f78530170aaae216b9347cf94c0e51a 
>   keduvocdocument/CMakeLists.txt c242c46fa79f128d63ccdeb5c4f611ad7294c874 
>   keduvocdocument/keduvocarticle.h b0b69c06d3d91ecc1680ecb18c9dd074beed1cb1 
>   keduvocdocument/keduvoccsvreader.cpp e3d471c4ecbb1236bae73714f27e357a1681ec70 
>   keduvocdocument/keduvoccsvwriter.cpp 32172e47d3617c082378969735dbd23d3178f06e 
>   keduvocdocument/keduvocdocument.h 9da14c62c84a07ffc53ef46f4c48e35824210d2c 
>   keduvocdocument/keduvocdocument.cpp 7026ccb9046968821b11ce6e07276d81e58e41c6 
>   keduvocdocument/keduvockvtml2reader.cpp 3884724422c318e34a5b8efc5e05481e27f7c6bc 
>   keduvocdocument/keduvockvtmlcompability.cpp e486729054a9e69fd52c4e4f1031a0bdbb3433cf 
>   keduvocdocument/keduvocpaukerreader.cpp 4ded3e480222ea9401b8cbfddad2b99ba82036fb 
>   keduvocdocument/keduvoctranslation.h c41d293755f0df7f87a41300569b093d020beb06 
>   keduvocdocument/keduvocvokabelnreader.cpp ffbd64d3cd50e519af319d6d52a1491a7ed1588a 
>   keduvocdocument/keduvocwqlreader.cpp 0841b1054ec41b93f4fffb1165c2c0d51806aeae 
>   keduvocdocument/keduvocxdxfreader.cpp 4317f6ff1fbc8a4a19c2bbcc72af2ace295743b6 
>   keduvocdocument/sharedkvtmlfiles.cpp 9f74aa2af2f69f25fca8efa1585b8c8ea1127cd9 
>   keduvocdocument/tests/CMakeLists.txt ac01a2b4a3e3b925517b2f004feb93bf365344a7 
>   keduvocdocument/tests/converter.cpp 1916b2269a88e932beb46f165e6cebebcefecb3c 
>   keduvocdocument/tests/sharedkvtmlfilestest.cpp 489dad4fcafa0a97fe4e93905206fb0fc53e0cf5 
> 
> Diff: https://git.reviewboard.kde.org/r/118970/diff/
> 
> 
> Testing
> -------
> 
> It still builds and the tests run.
> 
> Unrelated the sharedkvtmlfilestest exposes a bug in KEduVocDocument::open where the KFilterDev wont open, (bug from previous commit) not sure what to do here to fix it though.
> 
> 
> Thanks,
> 
> Jeremy Whiting
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20140627/3e170af3/attachment.html>


More information about the kde-edu mailing list