[LLVMdev] code-owner sporks

Sean Silva silvas at purdue.edu
Fri Nov 16 17:25:13 PST 2012


> code blocks in llvm-commits email.  At this point we really don't know
> if one solution is better than the other, but we have good reason to
> believe Pull Requests might be a big win.

I feel the opposite, TBH. All of LLVM's development history to date
points to llvm-commits being awesome with a long, proven history, and
pull requests are "unknown".

-- Sean Silva

On Fri, Nov 16, 2012 at 7:39 PM, Greg Fitzgerald <garious at gmail.com> wrote:
> David A. Green wrote:
>> I find llvm-commits daunting.  So much that I hesitate to do reviews.
>> As Chris commented, I am not very active on that list.  There's a reason
>> for that beyond lack of time.
>
> So the goal is to make it easier for a member of the community to
> review only commits to a sub-tree that interests them?
>
> Let's say it may or may not be easier for reviewers to monitor the
> Pull Requests of a spork than to write a clever filter for
> llvm-commits.  And we'll also say that it may or may not be easier for
> reviewers to comment on a patch on Github than trying to reference
> code blocks in llvm-commits email.  At this point we really don't know
> if one solution is better than the other, but we have good reason to
> believe Pull Requests might be a big win.  So rather than all the
> talk, can we baby-step forward in a noncommittal way?
>
> How about allowing the code owner to add a line to CODE_OWNERS.TXT of
> the location to submit patches?  If no location is given, assume
> llvm-commits.  If the URI is a Github spork, the contributor should
> make a Pull Request.
>
> Thanks,
> Greg
>
> On Fri, Nov 16, 2012 at 3:50 PM,  <dag at cray.com> wrote:
>> Sean Silva <silvas at purdue.edu> writes:
>>
>>>> - I have to *remember* I submitted the patch (not hard, but it is a
>>>>   cost).
>>>
>>> If you forgot, the chances are high that the patch was unimportant. I
>>> do my development on local git branches, so every time I do `git
>>> branch`, I'm reminded. There's really no overhead.
>>
>> Every time you have to check it's overhead.  It may be small overhead,
>> but it's nonzero.
>>
>>>> - I have to save that e-mail from llvm-commits so I can refer to it when
>>>>   the inevitable ping is necessary.
>>>
>>> Maybe you need a better mail-reader? In gmail I just look in my "sent
>>> mail" or if I have sent a lot of mail recently I search "is:sent
>>> has:attachment". It literally takes 10 seconds.
>>
>> Ok, I'll give you this one.  :)
>>
>>>> - I have to wade through tons and tons of commit e-mails searching for a
>>>>   response to my patch (for some reason the mailing list software often
>>>>   breaks threading).  This is a more general problem with the current
>>>>   review process, not strictly a timeliness issue.
>>>
>>> Sounds like you need a better mail reader. I have never heard of this
>>> even remotely being a problem in any way, at all, ever.
>>
>> Really?  You've never heard comments about the huge volume of mail on
>> llvm-commits?  I just read another message about it today.
>>
>>>> - I have to send a ping e-mail.
>>>
>>> Once again, this takes like what, 10 seconds? I find it highly
>>> unlikely that 10 seconds is a non-negligible amount of time compared
>>> to the time spent coding, building, and testing any non-obvious (i.e.
>>> needing review) change.
>>
>> Again, it's non-zero.  I have to break out of whatever I'm doing,
>> context switch and send the mail.
>>
>>>> - Now I have to look for responses to *two* e-mails.
>>>
>>> You definitely need a better mail reader. In every mail reader I've
>>> ever seen, both mails get clustered into a thread. Maybe this is the
>>> root cause of your vehement dissatisfaction with the current
>>> email-based system?
>>
>> Even if threads are clustered, one still has to look through tons of
>> subject lines to find it.
>>
>>> Have you considered the amount of quality code that gets produced with
>>> the current process?
>>
>> Code quality doesn't reflect how efficient the review system is.  It
>> reflects how good the reviewers are and we are fortunate to have
>> excellent ones.
>>
>> I find llvm-commits daunting.  So much that I hesitate to do reviews.
>> As Chris commented, I am not very active on that list.  There's a reason
>> for that beyond lack of time.  By definition, making the process simpler
>> and more organized should result in less time for reviews and thus
>> hopefully more review participation.
>>
>>> What kind of improvements can we expect from such a system?
>>
>> A one-stop shop for reviewers to find patches.  Something searchable by
>> subsystem, file name, etc.  I can't review every patch that goes by but
>> I certainly can review patches for files I'm currently working in.  If I
>> could start my day by doing a patch search on whatever file I'm
>> currently working on, I'd make it a daily practice to do some reviews.
>> Right now that is tedious to do with e-mail.
>>
>>> Are there advantages that email has over a more structured
>>> sort of review system like Phabricator?
>>
>> I don't know.  I would like to know if there are any.  I suppose it's
>> super convenient to open the list of messages but beyond that I get
>> overwhelmed.
>>
>>> Are you sure that the bottleneck
>>> which causes high review latency isn't elsewhere in the system?
>>
>> Of course I'm not sure.  I'm not sure anyone is sure.  :) All I know is
>> that there's a problem.
>>
>>> Or, as counterintuitive as it may seem, perhaps all of the things you
>>> are pointing to as problematic (e.g. review latency, patches getting
>>> dropped) are actually *beneficial* (maybe it prevents less-determined
>>> people from becoming contributors?
>>
>> If that's our goal then I will just say right out it is not a good one.
>>
>>                            -David
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



More information about the llvm-dev mailing list