Quantcast

python review help

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

python review help

Ammarguellat, Zahira via cfe-dev
Hi Anna and Devin,
Hello All,

I would like to get some help or more information to understand how to contribute to the scan-build part of this project.. As you might know I was developed a python rewrite of the existing scan-build tool which is intended to replace the Perl version. (This was mentioned on the open project page of clang analyzer.)

My first commit was merged a year before. Then I continued to develop the functionality out of the source tree. With agreement with Anna and Devin my focus was to port the functional tests to lit (the LLVM test tool). As the result of the new test suite, fair amount of bug was caught and fixed... Since December I'm trying to merge these improvements back to the Clang repo. With my first attempt I wanted all improvement merged once. After weeks of silence I got the feedback that it's too big. Fair enough. Now I'm trying to push my change in pieces which are could be understood better. Now again, I'm missing responses, approvals on these changes.

I'm looking for developers who can review my changes (and are familiar with Python constructs such: iterators, generators, decorators). Would anybody volunteer for reviewer? Anna, Devin: Would that be acceptable for you?

Thanks,
Laszlo

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: python review help

Ammarguellat, Zahira via cfe-dev
I volunteer to review your code. I'm well familiar with Python. It'll
be an exercise being new to clang, but ready I'm to do it. Please
email me and let's take it further.

On Sun, Feb 12, 2017 at 12:01 AM, Laszlo Nagy via cfe-dev
<[hidden email]> wrote:

> Hi Anna and Devin,
> Hello All,
>
> I would like to get some help or more information to understand how to
> contribute to the scan-build part of this project.. As you might know I was
> developed a python rewrite of the existing scan-build tool which is intended
> to replace the Perl version. (This was mentioned on the open project page of
> clang analyzer.)
>
> My first commit was merged a year before. Then I continued to develop the
> functionality out of the source tree. With agreement with Anna and Devin my
> focus was to port the functional tests to lit (the LLVM test tool). As the
> result of the new test suite, fair amount of bug was caught and fixed...
> Since December I'm trying to merge these improvements back to the Clang
> repo. With my first attempt I wanted all improvement merged once. After
> weeks of silence I got the feedback that it's too big. Fair enough. Now I'm
> trying to push my change in pieces which are could be understood better. Now
> again, I'm missing responses, approvals on these changes.
>
> I'm looking for developers who can review my changes (and are familiar with
> Python constructs such: iterators, generators, decorators). Would anybody
> volunteer for reviewer? Anna, Devin: Would that be acceptable for you?
>
> Thanks,
> Laszlo
>
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: python review help

Ammarguellat, Zahira via cfe-dev
In reply to this post by Ammarguellat, Zahira via cfe-dev

On Feb 11, 2017, at 11:01 PM, Laszlo Nagy <[hidden email]> wrote:

Hi Anna and Devin,
Hello All,

I would like to get some help or more information to understand how to contribute to the scan-build part of this project.. As you might know I was developed a python rewrite of the existing scan-build tool which is intended to replace the Perl version. (This was mentioned on the open project page of clang analyzer.)

My first commit was merged a year before. Then I continued to develop the functionality out of the source tree.

Hi Laszlo,

The LLVM Dev policy is focused around incremental development, where you submit small incremental patches as you are making improvements. Developing large changes out of the tree leads to various issues. For example, it’s more likely that a developer will waste time implementing things a certain way which the community/reviewers disagree with. Incremental development makes a feedback loop much shorter and avoids wasted effort. Another issue is that smaller changes are much easier to review and do not require a large time commitment from a reviewer at the specific time when the patch lands. Generally, the reviewers try to accommodate some constant flow of patch review into their schedule. 

Another problem is that the ratio of developers to reviewers is very low, mainly since fewer developers are willing to volunteer their time to code review. (This, unfortunately, leads to a bigger problem of patches across different areas of clang/LLVM not getting any reviews at all!) It is very hard to ensure that the few people who are willing to review a certain area of LLVM/clang have the time to review large patches or batches of patches developed over a period of months and landing all at once.

This and other rational behind the incremental development policy are outlined here http://llvm.org/docs/DeveloperPolicy.html#incremental-development. It’s also important to read other parts of the Developer Policy if you haven’t already.

With agreement with Anna and Devin my focus was to port the functional tests to lit (the LLVM test tool). As the result of the new test suite, fair amount of bug was caught and fixed... Since December I'm trying to merge these improvements back to the Clang repo. With my first attempt I wanted all improvement merged once. After weeks of silence I got the feedback that it's too big. Fair enough. Now I'm trying to push my change in pieces which are could be understood better. Now again, I’m missing responses, approvals on these changes.


I saw the following 6 patches sent for review at the end of January (not sure if there are more). Looks like 3 were reviewed and accepted by Devin within a day of you submitting them. I assume he just does not have the bandwidth to review all of them fast.


I'm looking for developers who can review my changes (and are familiar with Python constructs such: iterators, generators, decorators). Would anybody volunteer for reviewer? Anna, Devin: Would that be acceptable for you?


It would be great if someone is interested in helping out with review of the remaining patches and with code reviews for scan-build-py/libbear improvements in general!!!

+ Jonathan who was interested in this work and mentioned that he might have time to help out with code reviews.

Thanks,
Laszlo


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: python review help

Ammarguellat, Zahira via cfe-dev



On 2/13/17 12:28 AM, Anna Zaks wrote:

I saw the following 6 patches sent for review at the end of January (not sure if there are more). Looks like 3 were reviewed and accepted by Devin within a day of you submitting them. I assume he just does not have the bandwidth to review all of them fast.


I'm looking for developers who can review my changes (and are familiar with Python constructs such: iterators, generators, decorators). Would anybody volunteer for reviewer? Anna, Devin: Would that be acceptable for you?


It would be great if someone is interested in helping out with review of the remaining patches and with code reviews for scan-build-py/libbear improvements in general!!!

+ Jonathan who was interested in this work and mentioned that he might have time to help out with code reviews.

Yeah, feel free to CC me on reviews... I'm happy to contribute where I have time (probably on the order of an hour a week ish?).


Jon


Thanks,
Laszlo


-- 
Jon Roelofs
[hidden email]
CodeSourcery / Mentor Embedded

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: python review help

Ammarguellat, Zahira via cfe-dev
Thanks Jonathan for the quick actions and your offer.

Akhil, thanks for your offer. Please register yourself onto the phabricator, otherwise can't add you as a reviewer.

Anna, thanks for your reply. I agree with every word of the developer policy, but it was not working: When the response time is weeks for questions and reviews, I felt the out of source tree is a good solution. Might have been wrong. Now with more people on board, hope this will not be a blocker.

I'm happy to see that it progresses further.

Regards,
Laszlo

On Tue, Feb 14, 2017 at 1:50 AM, Jonathan Roelofs <[hidden email]> wrote:



On 2/13/17 12:28 AM, Anna Zaks wrote:

I saw the following 6 patches sent for review at the end of January (not sure if there are more). Looks like 3 were reviewed and accepted by Devin within a day of you submitting them. I assume he just does not have the bandwidth to review all of them fast.


I'm looking for developers who can review my changes (and are familiar with Python constructs such: iterators, generators, decorators). Would anybody volunteer for reviewer? Anna, Devin: Would that be acceptable for you?


It would be great if someone is interested in helping out with review of the remaining patches and with code reviews for scan-build-py/libbear improvements in general!!!

+ Jonathan who was interested in this work and mentioned that he might have time to help out with code reviews.

Yeah, feel free to CC me on reviews... I'm happy to contribute where I have time (probably on the order of an hour a week ish?).


Jon


Thanks,
Laszlo


-- 
Jon Roelofs
[hidden email]
CodeSourcery / Mentor Embedded


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Loading...