Review Request 111928: Make relative imports work in kdev-python

Sven Brauch svenbrauch at gmx.de
Wed Aug 7 09:52:50 UTC 2013


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

Ship it!


Looks good apart from the two nitpicks below, assuming you tested it (I didn't) ;)
Please run the unit tests and verify they all still pass ("make test").

Can you push this yourself, or do you want me to submit it?

Writing a unit test for such a feature is always very nice, since it guarantees it doesn't break and shouldn't be too difficult to do with the existing test framework. In case you want to add one you can send me another patch later or add it to this one.


duchain/contextbuilder.cpp
<http://git.reviewboard.kde.org/r/111928/#comment27562>

    Iterating over characters seems a bit pedantic, can't you use QString::indexOf('.', int) instead? I think that'd also be easier to read.



duchain/declarationbuilder.cpp
<http://git.reviewboard.kde.org/r/111928/#comment27563>

    please remove this


- Sven Brauch


On Aug. 7, 2013, 6:58 a.m., Zaar Hai wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111928/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2013, 6:58 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Details are in the bug.
> 
> 
> This addresses bug 322534.
>     http://bugs.kde.org/show_bug.cgi?id=322534
> 
> 
> Diffs
> -----
> 
>   duchain/contextbuilder.cpp 6f00f37 
>   duchain/declarationbuilder.cpp 37c5148 
> 
> Diff: http://git.reviewboard.kde.org/r/111928/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zaar Hai
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130807/7f995e0f/attachment.html>


More information about the KDevelop-devel mailing list