This is an automatically generated e-mail. To reply, visit: http://svn.reviewb= oard.kde.org/r/4320/

Regarding =
my previous comment about how you tested it: I realized KSambaShare is alre=
ady used by kdenetwork/kfilesharing, so I assume you tested this way. Still=
, a command-line client or even better unit tests would be a nice addition.=

= =

/trunk/KDE/kde= libs/kio/kio/ksambashare.cpp (Diff revisions 8 - 10)
QString KSambaSharePrivate::testParmParamValue(const QString& p= arameterName) const	QString KSambaSharePrivate::testparmParamValue(const QString &p= arameterName)
139	k= Debug() << "We got some errors";	128	k= Warning() <<= "We got some errors";

/trunk/KDE/kde= libs/kio/kio/ksambashare.cpp (Diff revisions 8 - 10)

QString KSambaSharePrivate::testParmParamValue(const QString& p=
arameterName) const

QString KSambaSharePrivate::testparmParamValue(const QString &p=
arameterName)

139

        k=
Debug() << "We got some errors";

128

        k=
Warning() <<=
 "We got some errors";

Please output stdErr in the warning until error handling is in place=
.

= =

/trunk/KDE/kde= libs/kio/kio/ksambashare.cpp (Diff revisions 8 - 10)
QByteArray KSambaSharePrivate::getNetUserShareInfo() const
161	k= Debug() << "We got some errors";	150	k= Warning() <<= "We got some errors";

Output stderr as well.

= =

/trunk/KDE/kde= libs/kio/kio/ksambashare.cpp (Diff revisions 8 - 10)
bool KSambaSharePrivate::sync()
322	kDebug() &l= t;< "something wrong!";	273	kWarning() = << "something wrong!";

Outputting the name of the key here could be helpful.

= =

/trunk/KDE/kde= libs/kio/kio/ksambashare.cpp (Diff revisions 8 - 10)
bool KSambaSharePrivate::sync()
		283	delete data.valu= e(name);

Nitpick: you can simplify this with:
delete data.take(name); (avoids going through the hash twice)

= =

/trunk/KDE/kdelib= s/kio/kio/ksambashare.h (Diff revision 10)

		77	QList<KSambaShareData *> getShareByPath(const QString &path) const;

Should be named getShare*s*ByPath() since it returns a list of share=
s.

= =

/trunk/KDE/kdeli= bs/kio/kio/ksambashare.h (Diff revision 10)

77	KSambaSharePrivate = * const d;	101	static KSambaShare *_ins= tance;

See my comment in KSambaShare::instance(). Unless there is a good re=
ason for this static to be there, it should go away (private static variabl=
es are of no use to class users anyway).

= =

/trunk/KDE/kdeli= bs/kio/kio/ksambashare.cpp (Diff revision 10)

250	K_GLOBAL_STATIC(KSambaShare, _instance)	353	if (_instance =3D=3D 0)

K_GLOBAL_STATIC is prefered over instantiating the singleton by hand=
. Is there a good reason for getting rid of it?

- Aur=C3=A9lien

On September 30th, 2010, 2:01 a.m., Rodrigo Belem wrote:

Review request for kdelibs, Raphael Kubo da Costa, Jonathan Thomas, Au= r=C3=A9lien G=C3=A2teau, Jonathan Riddell, Adenilson Cavalcanti, and lourei= ro.

By Rodrigo Belem.

Updated 2010-09-30 02:01:48

Descripti= on

KDE needs to support modern samba tools. With the "net =
usershare" command line tool the users can manage their shares. The at=
tached patch intends to add support for this tool.

Diffs=

/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)
/trunk/KDE/kdelibs/kio/CMakeLists.txt (118= 0108)
/trunk/KDE/kdelibs/kio/kio/ksambashare.h (= 1180108)
/trunk/KDE/kdelibs/kio/kio/ksambashare.cpp (1180108)

View Diff