Review Request 112702: KIO slavebase listEntry improvement and cleanups

[David Faure]

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



kioslave/file/file_unix.cpp
<http://git.reviewboard.kde.org/r/112702/#comment29609>

    seems to me that it should be indented even more?
    
    
    Anyhow, yes, commit the indentation fixes separately (without a new reviewboard request)



staging/kio/src/core/slavebase.h
<http://git.reviewboard.kde.org/r/112702/#comment29610>

    I'm pretty sure it was included already, via one of the kio/* includes below. You can just remove this.



staging/kio/src/core/slavebase.h
<http://git.reviewboard.kde.org/r/112702/#comment29611>

    "... and simply remove the call to listEntry(entry, true)"
    
    To be very clear about what people should do when porting.



staging/kio/src/core/slavebase.h
<http://git.reviewboard.kde.org/r/112702/#comment29612>

    (I know this was there before, but)
    
    This is true for all methods in SlaveBase. Let's remove the line, it's not "internal" for kioslave authors (and apps can't use SlaveBase, so no problem).



staging/kio/src/core/slavebase.cpp
<http://git.reviewboard.kde.org/r/112702/#comment29613>

    Not useful here. We're done with the current listDir call. The next one could be in a very long time (and surely starts the timer again, right?).



staging/kio/src/core/slavebase.cpp
<http://git.reviewboard.kde.org/r/112702/#comment29616>

    can be removed too
    
    (I know the old code was doing that, but only as an unnecessary side effect of sharing code between "send intermediate batch" and "send last batch")



staging/kio/src/core/slavebase.cpp
<http://git.reviewboard.kde.org/r/112702/#comment29615>

    yep, we do restart the timer when necessary  anyway (this answers my above question)



staging/kio/src/core/slavebase.cpp
<http://git.reviewboard.kde.org/r/112702/#comment29614>

    const UDSEntry &entry


- David Faure


On Sept. 12, 2013, 6:26 p.m., Mark Gaiser wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112702/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2013, 6:26 p.m.)
> 
> 
> Review request for KDE Frameworks and kdelibs.
> 
> 
> Description
> -------
> 
> This patch actually does two things:
> 1. A small indentation fix that this diff doesn't show properly (i can commit that as a separate commit if that's preferred)
> 2. A cleanup in how a listDir is handled. It used to be required to send a listEntry(UDSEntry, true) to indicate that the listing is completed. Then the UDSEntry was skipped. It seemed vague to me so i deprecated that function and added a new listEntry that doesn't take a bool. Now you just have to call finished() when done listing. finished() was already required otherwise KIO would blame you with warnings :)
> 
> 
> Diffs
> -----
> 
>   kioslave/file/file_unix.cpp 48bd0ba 
>   staging/kio/src/core/slavebase.h b46631e 
>   staging/kio/src/core/slavebase.cpp 2e46985 
> 
> Diff: http://git.reviewboard.kde.org/r/112702/diff/
> 
> 
> Testing
> -------
> 
> It compiles and runs just fine. The few tests that are there are passing though i doubt they test this case. the KIO file slave also works just fine. Compilation does give me new (expected_ deprecated errors for at least the http slave. I don't know how to test that one. It should just work fine though.
> 
> 
> Thanks,
> 
> Mark Gaiser
> 
>

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