[lldb-dev] large patches vs incremental changes

[jingham at apple.com]

> On Jul 1, 2014, at 12:01 PM, Zachary Turner <zturner at google.com> wrote:
> 
> I wanted to open a discussion about the policy regarding patch submission.
> 
> LLVM, and most of its other subprojects, have adopted a policy of making small, incremental changes, as outlined here [http://llvm.org/docs/DeveloperPolicy.html#making-a-major-change].   It seems that either we don't have this on LLDB, or we do have it and it's just not enforced.
> 
> I personally have a strong preference for the incremental approach.  All the reasons are outlined at the above link, so I don't think I need to repeat them.  Is it possible to move towards this model with LLDB?
 
I'm of two minds about the "incremental approach".  If I'm trying to make a structural change and I'm not sure how I'm going to do it, I want to try a bunch of stupid attempts without having to explain why this or that one seems good.  Then by the time I've figured out what I want to do the patch is pretty much ready, and going back and breaking it into stages seems to me make-work.  But that is my style, other people work differently.  Since we're getting free work out of folks (more or less) I shy away from imposing my style on others.  On the other hand, anything that is naturally separable should be separated out into separate patches.

> Additionally, I frequently see patches going in without review.  What is LLDB's policy on this?  Are we're ok with having broken code upstreamed for a short time on the condition that the committer is acting in good faith to fix it as soon as possible?  For example, I've had a patch up for 6 days that I've pinged on a couple of times, with no traction.  I've been waiting because I was under the impression that getting a review was a requirement.  But sometimes I see patches that seem non-trivial getting pushed straight to the server without any kind of review.  So I'm not sure if my understanding is correct.  Either way, some clarification woudl be nice.
> 

Many of the "unreviewed patches" you see going in are folks committing to code that they are the direct authors of.  In that case, gdb had a sometimes useful notion of "area owners" who could check in code to their areas without review.  But if you want to go to areas you know less about then you should ask for guidance, and if you haven't yet submitted some critical mass of "good patches" then your patches should wait on approval.  Some of that asking for guidance you don't see because for most of its life contributions to the core of lldb have come from folks who share a hallway.  We should get better about reflecting that through the mailing lists.

My general impression from working on gdb for many years, and of watching the gcc & llvm compiler communities, is that compilers are a "cool" project (it seems every CS student has built a compiler at some point and even if they haven't they have opinions about them) and there's generally enough interest in adding patches, making changes, etc, and from enough competing directions, that you need to have some more or less strict rules to avoid chaos.  Debuggers are much less cool, and the problem is less throttling change than getting folks to work on it at all.  To that end I'd prefer being more laissez faire till events show the need.

Jim

> Thanks!
> _______________________________________________
> lldb-dev mailing list
> lldb-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev