Review Request: Adding "net usershare" suport for KSambaShare

[Raphael Kubo da Costa]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/4320/#review9079
-----------------------------------------------------------


dfaure is still the best person to comment on the KDirWatch bits. As for the rest, it looks mostly fine, with these small pieces needing some work.


/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://svn.reviewboard.kde.org/r/4320/#comment9874>

    Several @return statements here are followed by a @c, even when the following word does not need to use a typewriter font.



/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://svn.reviewboard.kde.org/r/4320/#comment9875>

    Test -> Tests, for consistency. Missing '.' at the end of the sentence.



/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://svn.reviewboard.kde.org/r/4320/#comment9876>

    If you want to keep the @c's here, false needs one too.



/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://svn.reviewboard.kde.org/r/4320/#comment9877>

    an empty string



/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://svn.reviewboard.kde.org/r/4320/#comment9884>

    If this is deprecated, you can add KDE_DEPRECATED here.



/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://svn.reviewboard.kde.org/r/4320/#comment9878>

    Not sure if this apidox bit will actually be parsed and shown in api.kde.org



/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://svn.reviewboard.kde.org/r/4320/#comment9879>

    This variable can be removed:
    
    for (...) {
      if (condition) {
        foo();
        return true;
      }
    }
    
    ...
    
    return false;



/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://svn.reviewboard.kde.org/r/4320/#comment9880>

    if (...) {
      return QString::fromLocal8Bit(...);
    }
    
    return QString();
    
    and then we can get rid of result.



/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://svn.reviewboard.kde.org/r/4320/#comment9881>

    const QFileInfo pathInfo(path);



/trunk/KDE/kdelibs/kio/kio/ksambasharedata.h
<http://svn.reviewboard.kde.org/r/4320/#comment9885>

    A brief apidox description + @since 4.6 would be good to have here.



/trunk/KDE/kdelibs/kio/kio/ksambasharedata.h
<http://svn.reviewboard.kde.org/r/4320/#comment9882>

    Nitpick: use all verbs in the 3rd person?



/trunk/KDE/kdelibs/kio/kio/ksambasharedata.cpp
<http://svn.reviewboard.kde.org/r/4320/#comment9883>

    I _think_ we don't.


- Raphael


On 2010-12-01 01:56:04, Rodrigo Belem wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/4320/
> -----------------------------------------------------------
> 
> (Updated 2010-12-01 01:56:04)
> 
> 
> Review request for kdelibs, Raphael Kubo da Costa, Jonathan Thomas, Aurélien Gâteau, Jonathan Riddell, Adenilson Cavalcanti, loureiro, and Daniel Nicoletti.
> 
> 
> Summary
> -------
> 
> KDE needs to support modern samba tools. With the "net usershare" command line tool the users can manage their shares. The attached patch intends to add support for this tool.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/includes/CMakeLists.txt 1202449 
>   /trunk/KDE/kdelibs/includes/KSambaShareData PRE-CREATION 
>   /trunk/KDE/kdelibs/kio/CMakeLists.txt 1202449 
>   /trunk/KDE/kdelibs/kio/kio/ksambashare.h 1202449 
>   /trunk/KDE/kdelibs/kio/kio/ksambashare.cpp 1202449 
>   /trunk/KDE/kdelibs/kio/kio/ksambashare_p.h PRE-CREATION 
>   /trunk/KDE/kdelibs/kio/kio/ksambasharedata.h PRE-CREATION 
>   /trunk/KDE/kdelibs/kio/kio/ksambasharedata.cpp PRE-CREATION 
>   /trunk/KDE/kdelibs/kio/kio/ksambasharedata_p.h PRE-CREATION 
> 
> Diff: http://svn.reviewboard.kde.org/r/4320/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Rodrigo
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20101201/9b7d75b6/attachment.htm>