Review Request: Tests for GDLHandler

[Laszlo Papp]

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


I tested it myself, and at least it did not fail locally. :)


core/tests/gdlhandlertest.h
<http://git.reviewboard.kde.org/r/102511/#comment5637>

    Could you please use imperative as in "Return" as you did in the previous patch, if I am not mistaken ?



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5636>

    I personally prefer pre-incrementing to post, even if it does not have any gain in this special case. I use that everywhere to make the codebase consistent in usage.
    
    Since you have already used pre-incrementing few lines above in a very similar situation, you should change one of them to make consistent after all. Please use pre-increment here. :)



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5632>

    Could you please use read-only index access here (since you just actually read it) ? nameList1.at(i) ...



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5633>

    ...at(i)...



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5634>

    read access to the index.



core/tests/gdlhandlertest.cpp
<http://git.reviewboard.kde.org/r/102511/#comment5635>

    no push_back, but append. :)


- Laszlo


On Sept. 7, 2011, 6:42 p.m., Ashwin Rajeev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102511/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2011, 6:42 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Add few new test cases in gdlhandlertest
> {ensureParsing,ensureSerializing,ensureCommenting,testGDLSample,testCommentAtBegin,testCommentAtEnd}
>     
> Improve methods already in gdlhandler 
> 
> 
> Diffs
> -----
> 
>   core/tests/gdlhandlertest.h b0e3f72 
>   core/tests/gdlhandlertest.cpp c492a5d 
> 
> Diff: http://git.reviewboard.kde.org/r/102511/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ashwin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gluon/attachments/20110908/8b4a8ee6/attachment-0001.html>