Review Request 112463: Port SMB kioslave to KF5/Qt5

Mark Gaiser markg85 at gmail.com
Tue Dec 10 17:02:31 GMT 2013



> On Dec. 10, 2013, 3:54 p.m., Aleix Pol Gonzalez wrote:
> > kioslave/smb/kio_smb_internal.cpp, line 68
> > <http://git.reviewboard.kde.org/r/112463/diff/3/?file=222942#file222942line68>
> >
> >     Does it really need QDir::separator()? It seems to me like everything should be '/'.

"could" be, but i'm not quite sure. The path() function doesn't add a trailing slash if there is none.
So that probably leaves the question: "Does the current path always have a trailing slash?". I don't know. I'm better safe then sorry and just leave the QDir::separator() unless you are 100% sure i can manage without it?


> On Dec. 10, 2013, 3:54 p.m., Aleix Pol Gonzalez wrote:
> > kioslave/smb/kio_smb_internal.h, line 35
> > <http://git.reviewboard.kde.org/r/112463/diff/3/?file=222941#file222941line35>
> >
> >     it might be a good moment to remove this comment-

:) will do


> On Dec. 10, 2013, 3:54 p.m., Aleix Pol Gonzalez wrote:
> > kioslave/smb/kio_smb_auth.cpp, line 76
> > <http://git.reviewboard.kde.org/r/112463/diff/3/?file=222937#file222937line76>
> >
> >     AAAAAAAAA
> >     
> >     xD
> >     Maybe we can just remove this one?

Just the "AAAAAAAAA" i suppose? So the rest can stay?


> On Dec. 10, 2013, 3:54 p.m., Aleix Pol Gonzalez wrote:
> > kioslave/smb/kio_smb.h, line 48
> > <http://git.reviewboard.kde.org/r/112463/diff/3/?file=222935#file222935line48>
> >
> >     do you need klocale? it's deprecated.

Interesting! I'm not getting any deprecated notices from locale based function so i wonder if any functions from that include are used at all. Will take a look at it and remove it if possible.


- Mark


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112463/#review45466
-----------------------------------------------------------


On Dec. 5, 2013, 11:44 p.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112463/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2013, 11:44 p.m.)
> 
> 
> Review request for KDE Runtime and KDE Frameworks.
> 
> 
> Repository: kde-runtime
> 
> 
> Description
> -------
> 
> This is the initial port! I added two TODO lines in the diff for parts where i'm not sure if I've ported them correctly.
> Also, i needed a change in FindSamba.cmake to even get the samba detection working. That reviewrequest is waiting here: https://git.reviewboard.kde.org/r/112448/ you're probably OK if you still use samba 3.x
> 
> Once i know that this is actually working then i will comment some qDebug lines.
> 
> 
> Diffs
> -----
> 
>   kioslave/CMakeLists.txt fc594e4 
>   kioslave/smb/CMakeLists.txt a3a2265 
>   kioslave/smb/kio_smb.h c2229ab 
>   kioslave/smb/kio_smb.cpp 2c2523a 
>   kioslave/smb/kio_smb_auth.cpp 4d236b4 
>   kioslave/smb/kio_smb_browse.cpp 5253be9 
>   kioslave/smb/kio_smb_dir.cpp ba80c86 
>   kioslave/smb/kio_smb_file.cpp 206526a 
>   kioslave/smb/kio_smb_internal.h 4b946c1 
>   kioslave/smb/kio_smb_internal.cpp e943844 
>   kioslave/smb/kio_smb_mount.cpp a5a7e8e 
>   kioslave/smb/kio_smb_win.h f1dcb6f 
>   kioslave/smb/kio_smb_win.cpp 14dd797 
> 
> Diff: http://git.reviewboard.kde.org/r/112463/diff/
> 
> 
> Testing
> -------
> 
> It compiles and gets loaded just fine. I tried testing this on an actual samba share, but i kept getting a 111 error (connection refused) from kio_smb so i'm hoping that is a local issue here. If someone else could try this out and verify that it's either working or broken.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

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


More information about the kde-core-devel mailing list