Review Request 114421: Fix binary operations on unsure types.

Sven Brauch svenbrauch at googlemail.com
Thu Dec 12 20:32:22 UTC 2013


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



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32554>

    Set kate to "Remove spaces: On modified lines" in Settings -> Open/Save.
    Empty lines should not have spaces (I know some of kdev-python still has it but I'm fighting it ;)



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32553>

    remove this



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32555>

    here and everywhere below: always use { }, always use a newline



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32556>

    don't abbreviate the "Structure", the name is hard enough to understand with the rhs ;)



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32557>

    just return Helper::...



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32558>

    here and below: it's visitOr not visitEr



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32559>

    move this line to just before the if



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32560>

    remove this, you don't use the declaration so you don't need the check.
    Instead you might want to add a check for the type. It should not be null but it might. You can just put that into the if, like if ( type && type->whichType() == ... )



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32561>

    inconsistent formatting: missing spaces before and after ( and )



duchain/expressionvisitor.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32562>

    The indent here is totally broken.



duchain/tests/pyduchaintest.h
<http://git.reviewboard.kde.org/r/114421/#comment32563>

    please rename the method to something like testBinaryOperatorsUnsure() or so.



duchain/tests/pyduchaintest.cpp
<http://git.reviewboard.kde.org/r/114421/#comment32564>

    Can you add a few more examples? E.g. class with the relevant operator not defined; two classes with the relevant operator not defined; what you have, but reversed; etc


- Sven Brauch


On Dec. 12, 2013, 8:07 p.m., Levente Kurusa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114421/
> -----------------------------------------------------------
> 
> (Updated Dec. 12, 2013, 8:07 p.m.)
> 
> 
> Review request for KDevelop and Sven Brauch.
> 
> 
> Repository: kdev-python
> 
> 
> Description
> -------
> 
> This patch is a GCI task.
> 
> http://www.google-melange.com/gci/task/view/google/gci2013/5282511542288384
> 
> 
> Diffs
> -----
> 
>   duchain/expressionvisitor.h 31c6afb 
>   duchain/expressionvisitor.cpp 082f895 
>   duchain/tests/pyduchaintest.h 0b6413b 
>   duchain/tests/pyduchaintest.cpp 5f67716 
> 
> Diff: http://git.reviewboard.kde.org/r/114421/diff/
> 
> 
> Testing
> -------
> 
> Attached unit tests all pass.
> 
> 
> Thanks,
> 
> Levente Kurusa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20131212/a05b5a86/attachment-0001.html>


More information about the KDevelop-devel mailing list