Review Request: Tests for GDLHandler

[Laszlo Papp]

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



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

    why not follow the previous gluonObject1/2 schema we started doing in order to make more talkative ?



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

    It is not the addition of this patch, but I would like to warn you: please avoid push_back stl style statement, and use append which read better and does not conflict with the general camelCase principles, so the naming remains nicer. :)
    
    Also, the identation is not good here.



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

    append instead of "push_back" here, too.



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

    I removed this static modifier in master sinec it is not needed. Could you also please do the same with the remainings ?
    
    I think this static was done that way in the original code on gitorious before I started integrated, thus it was just a left-over.



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

    Please use talkative name instead of "t" here as well



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

    Talkative names here as well, please.



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

    More talkative name accordingly.



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

    More talkative name accordingly.



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

    More talkative name accordingly.



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

    line break after the end of this logical unit.



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

    More talkative name accordingly.



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

    More talkative name accordingly.



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

    More talkative name accordingly.



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

    More talkative name accordingly.



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

    More talkative name accordingly.



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

    More talkative name accordingly.



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

    Line break after the logical unit



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

    More talkative name accordingly.



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

    Line break after the logical unit



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

    More talkative name accordingly.



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

    Line break after the logical unit



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

    why not parentObject and childObject as in other patch(es) ?



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

    I would personally be using "== true/false" for being explicit in test cases.



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

    gluonObject variable name would be better, the usage and the name accordingly.



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

    same, I would be explicit.



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

    gluonObjectList ?



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

    gluonObject variable name would be better, the usage and the name accordingly.



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

    Same, I would be explicit.



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

    Change the AnotherObject naming accordingly please.


- Laszlo


On Sept. 1, 2011, 6:33 p.m., Ashwin Rajeev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102511/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2011, 6:33 p.m.)
> 
> 
> Review request for Gluon.
> 
> 
> Summary
> -------
> 
> Add few new test cases in gdlhandlertest
> {ensureParsing,ensureSerializing,ensureCommenting,testGDLSample,testCommentAtBegin,testCommentAtEnd}
>     
> Modify compareTrees method to check children of gluonobjects
> 
> 
> Diffs
> -----
> 
>   core/tests/gdlhandlertest.h 94a33c9 
>   core/tests/gdlhandlertest.cpp 90ac380 
> 
> 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/20110906/5f63dbc8/attachment-0001.html>