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

Sven Brauch svenbrauch at gmx.de
Wed Aug 7 15:31:53 UTC 2013



> On Aug. 7, 2013, 9:52 a.m., Sven Brauch wrote:
> > 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.
> 
> Zaar Hai wrote:
>     I'll be happy to push, but I obviously do not have permissions. Also small guideline on how to push code to KDE would be useful.

Ok, in that case I'll push it. If you intend to continue contributing code to KDE, you can consider applying for a contributor account. You will need I think two people which already have a contributor account vouching for you.


> On Aug. 7, 2013, 9:52 a.m., Sven Brauch wrote:
> > duchain/contextbuilder.cpp, line 342
> > <http://git.reviewboard.kde.org/r/111928/diff/1/?file=176537#file176537line342>
> >
> >     Iterating over characters seems a bit pedantic, can't you use QString::indexOf('.', int) instead? I think that'd also be easier to read.
> 
> Zaar Hai wrote:
>     I do not understand - I'm iterating "name" until I stop seeing dots. And for each dot I encounter I perform action. Single indexOf will not do it if this is what you are referring to.

Well, I'd have gone for something like while ( pos = tname.indexOf('.', pos) != -1 ) { ... }
But it doesn't matter, it's fine as it is.


- Sven


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


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/872befa1/attachment.html>


More information about the KDevelop-devel mailing list