[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review needed: Add support of lzma in KDE
From:       "=?ISO-8859-1?Q?Nicolas_L=E9cureuil?=" <neoclust.kde () gmail ! com>
Date:       2008-08-24 11:40:26
Message-ID: 547338e40808240440x3a54dd87mf227aab27619f2d5 () mail ! gmail ! com
[Download RAW message or body]

On Sun, Aug 24, 2008 at 9:26 AM, Thiago Macieira <thiago@kde.org> wrote:
> Nicolas L=E9cureuil 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
>>=D8yvind 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.

["signature.asc" (text/plain)]

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iD8DBQBIsQ0NM/XwBW70U1gRAtRmAJwIzRWFVuazd3Gtw9yUH0+mRYvV4gCfQ2yb
Df73AeTdlqsu+EGAYIGTTw0=
=eiEC
-----END PGP SIGNATURE-----


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic