Review Request: Adding "net usershare" suport for KSambaShare
Andreas Hartmetz
ahartmetz at gmail.com
Wed Jun 16 23:37:14 BST 2010
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4320/#review6157
-----------------------------------------------------------
The coding style also seems to be inconsistent regarding braces/no braces and indentation of braces - since this is kdelibs code, please just use kdelibs style.
The points below are from a superficial review...
/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://reviewboard.kde.org/r/4320/#comment5784>
Did you mean: GuestOptions? (see below)
/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://reviewboard.kde.org/r/4320/#comment5783>
how do you want to split it? In any case later changes should be avoided after adding public API, so better think about it again.
/trunk/KDE/kdelibs/kio/kio/ksambashare.h
<http://reviewboard.kde.org/r/4320/#comment5782>
guestOk is of type GuestOptions and will be a bad name should GuestOptions ever be extended beyond basically "yes" and "no". It is already not a very good name now.
/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://reviewboard.kde.org/r/4320/#comment5779>
this could be clearer by doing the variable assignment which is the whole purpose of the loop right here.
/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://reviewboard.kde.org/r/4320/#comment5780>
QLatin1String("Yes")
/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://reviewboard.kde.org/r/4320/#comment5781>
why suddenly 2-spaces indentation?
/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://reviewboard.kde.org/r/4320/#comment5776>
return result == 0;
/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp
<http://reviewboard.kde.org/r/4320/#comment5777>
return result == 0;
- Andreas
On 2010-06-15 20:19:09, Rodrigo Belem wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4320/
> -----------------------------------------------------------
>
> (Updated 2010-06-15 20:19:09)
>
>
> Review request for kdelibs, Aurélien Gâteau, Jonathan Riddell, and Adenilson Cavalcanti.
>
>
> 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/kio/kio/ksambashare.h 1137974
> /trunk/KDE/kdelibs/kio/kio/ksambashare.cpp 1137974
>
> Diff: http://reviewboard.kde.org/r/4320/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Rodrigo
>
>
More information about the kde-core-devel
mailing list