Review Request: Added support for LZO archives

[Raphael Kubo da Costa]

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


Given the problems we have with file addition/deletion and the fact that you prefer to use the command-line program for now, I think this plugin should be read-only. However, this means cliinterface should also be aware of read-only programs. You could either work on that or try to use liblzo directly if you prefer.

You also need to plug this code into the buildsystem.


plugins/clilzoplugin/cliplugin.h
<http://git.reviewboard.kde.org/r/103100/#comment7630>

    "incontent" still looks weird. At least it should have been camelCased.



plugins/clilzoplugin/cliplugin.cpp
<http://git.reviewboard.kde.org/r/103100/#comment7628>

    This is a variable local to the method, so you should not use the same naming scheme we use for class attributes. Plus it should be declared when it is used. And "splitted" is not a word in English, "splitPath" would sound better :)



plugins/clilzoplugin/cliplugin.cpp
<http://git.reviewboard.kde.org/r/103100/#comment7625>

    This can be const, and use a camelCasedName, such as "fileProperties".



plugins/clilzoplugin/cliplugin.cpp
<http://git.reviewboard.kde.org/r/103100/#comment7626>

    No need to use spaces after QLatin1String()



plugins/clilzoplugin/cliplugin.cpp
<http://git.reviewboard.kde.org/r/103100/#comment7627>

    No need to surround QLatin1Char() with spaces.



plugins/clilzoplugin/cliplugin.cpp
<http://git.reviewboard.kde.org/r/103100/#comment7629>

    myList.last()?



plugins/clilzoplugin/kerfuffle_clilzo.desktop
<http://git.reviewboard.kde.org/r/103100/#comment7623>

    You still shouldn't add the translations yourself, they will be overwritten by the translation teams.



plugins/clilzoplugin/kerfuffle_clilzo.desktop
<http://git.reviewboard.kde.org/r/103100/#comment7624>

    You need to specify the mimetypes in the CMakeLists.txt since commit f8a2d09e7c921c933365c80b8ed5d51e8bf124e7.


- Raphael Kubo da Costa


On Nov. 26, 2011, 2:37 p.m., Othmane Moustaouda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103100/
> -----------------------------------------------------------
> 
> (Updated Nov. 26, 2011, 2:37 p.m.)
> 
> 
> Review request for KDE Utils and Raphael Kubo da Costa.
> 
> 
> Description
> -------
> 
> Added support for LZO archives using lzop utility
> 
> 
> This addresses bug 211669.
>     http://bugs.kde.org/show_bug.cgi?id=211669
> 
> 
> Diffs
> -----
> 
>   plugins/clilzoplugin/cliplugin.h PRE-CREATION 
>   plugins/clilzoplugin/cliplugin.cpp PRE-CREATION 
>   plugins/clilzoplugin/kerfuffle_clilzo.desktop PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/103100/diff/diff
> 
> 
> Testing
> -------
> 
> Listing and adding files to a new archive are working. There are some problems with deleting (lzop does not support it) and
> with adding files to an existing archive.
> 
> lzop does not fit perfectly with CLI interface, now I'm looking at how to change something to make it more elastic.
> 
> 
> Thanks,
> 
> Othmane Moustaouda
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20111223/85d89309/attachment-0001.html>