> 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 > >
