Review Request: Adding "net usershare" suport for KSambaShare

[Rodrigo Belem]

> On 2010-11-17 11:05:16, David Faure wrote:
> > /trunk/KDE/kdelibs/kio/kio/ksambashare.h, line 60
> > <http://svn.reviewboard.kde.org/r/4320/diff/14/?file=41413#file41413line60>
> >
> >     Missing "@since 4.6" in the documentation (for all new methods)

I promise that when no more changes to the code were needed I will write the docs :-)


> On 2010-11-17 11:05:16, David Faure wrote:
> > /trunk/KDE/kdelibs/kio/kio/ksambashare.h, line 74
> > <http://svn.reviewboard.kde.org/r/4320/diff/14/?file=41413#file41413line74>
> >
> >     Can you explain the difference with sharedDirectories()? 
> >     It only contains the name of the subdir in /var/lib/samba/usershares? Is this useful, given that one has to know about this base path to be able to use the share in any way?
> >     
> >     I would be very careful with exposing more implementation details (just like smbConfPath is a problem now that you're not using smb.conf, exposing subdir names is specific to the 'net usershare' solution). Better have an API that does not depend on the implementation - like sharedDirectories() for instance, which works fine in both cases.
> >     
> >     Or am I missing something here?

sharedDirectories() lists the directories shared by the cmd line app "net usershare", e.g. net usershare add shared_folder /home/user/shared_folder; net usershare info.

I would like to remove the smbConfPath(), since it will not be used.


> On 2010-11-17 11:05:16, David Faure wrote:
> > /trunk/KDE/kdelibs/kio/kio/ksambashare.h, line 97
> > <http://svn.reviewboard.kde.org/r/4320/diff/14/?file=41413#file41413line97>
> >
> >     You can't rename a signal, that's incompatible!!
> >     Please revert to changed(), as bad as the name was.
> >     
> >     If you really want to, you can add a new signal, mark the old one as deprecated, and emit both, but in this case I'm not sure it's worth the trouble.
> >     
> >     
> >     Ah -- actually the docu is wrong too, isn't it? From the code it seems to me that your shareChanged() signal is emitted when a file is added/removed/modified in the usershare directory? Is this useful? (I mean if all changes of sharing are done in this code, then you could just emit shareChanged without using KDirWatch, no?)
> >     
> >     My suggestion:
> >     * Rename the signal back to change() to preserve compatibility.
> >     * Re-add the KDirWatch on smb.conf, for the benefit of applications relying on this.
> >     * Also emit the same signal when adding/removing shares, directly (no need for KDirWatch::addDir for that).

Watching the changes in /var/lib/samba/usershares is useful. When adding, removing or changing a share using `net usershare` it adds, removes or changes a file in that directory. This file is a configfile in the ini format.

I think that watching smb.conf is not needed anymore.


- Rodrigo


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


On 2010-11-17 00:46:45, Rodrigo Belem wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/4320/
> -----------------------------------------------------------
> 
> (Updated 2010-11-17 00:46:45)
> 
> 
> 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 1180108 
>   /trunk/KDE/kdelibs/kio/CMakeLists.txt 1180108 
>   /trunk/KDE/kdelibs/kio/kio/ksambashare.h 1180108 
>   /trunk/KDE/kdelibs/kio/kio/ksambashare.cpp 1180108 
>   /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/20101118/131a785c/attachment.htm>