Review needed: Add support of lzma in KDE

Nicolas Lécureuil neoclust.kde at gmail.com
Sun Aug 24 12:40:26 BST 2008


On Sun, Aug 24, 2008 at 9:26 AM, Thiago Macieira <thiago at kde.org> wrote:
> Nicolas Lécureuil wrote:
>>hi,
>>
>>in mandriva we use lzma to compress some tarballs, manpages, ... so we
>>needed to add support to lzma into KDE, which have been done by Per
>>Øyvind Karlsen
>>
>>Can someone review the two patches and tell me if i  can commit  them
>>into trunk ?
>>http://kenobi.mandriva.com/~neoclust/kdebase-add-lzma-support.patch
>>http://kenobi.mandriva.com/~neoclust/kdelibs-add-lzma-support.patch
>
> In kdelibs/kdecore/compression/klzmafilter.cpp, you have a #define with
> two underscores. Two underscores are reserved, so you must be changing
> some gcc and/or glibc specific configuration. Can you find a solution
> without that?

what   can be  be done is to replace is to replace
LZMA_VLI_VALUE_UNKNOWN by  (uint64_t)-1

> A few lines in the same file have tabulation, while most don't. That's
> probably unintentional. Before committing, please remove the tabs. (same
> for kdebase/runtime/nepomuk/strigibackend/sopranoindexwriter.cpp)
>

i will remove them

> In kdebase/runtime/doc/kioslave/lzma.docbook, you have
> <author>&Lauri.Watts; &Lauri.Watts.mail;</author>. Did Lauri really write
> that file or did you copy/paste from somewhere else? I don't know the
> policy here..

yes this is the real author

>
> In all the patch looks fine and I'd say commit.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: signature.asc
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080824/40ea6a36/attachment.asc>


More information about the kde-core-devel mailing list