<div dir="ltr">Hi Austin,<div><br></div><div>thanks for the explanation. All of this makes sense. I was just thinking that maybe it would make sense to have a couple more reviewers with a mandate to deal with cleanups quickly before you get to the juicier reviews, the ones that actually need attention / routing to interested parties. On the other hand, going through the queue once a day is pretty good, and if you think you can manage that, sounds great! (I am not just speaking for myself, but for the project and for other potential contributors - fast turnaround time is always motivating.)</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 30, 2014 at 9:38 PM, Austin Seipp <span dir="ltr"><<a href="mailto:austin@well-typed.com" target="_blank">austin@well-typed.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hey Gintautas,<br>
<br>
Yes, I apologize about that (and I missed this request in my quick<br>
read over this email yesterday). To be clear, I apologize if my<br>
review/merge latencies are too long. :) What normally happens it that<br>
I review and merge patches in bulk, about once or twice a week. I'll<br>
review about, say, a dozen patches one day, and wait a few days for<br>
more to come in, then sweep up everything in that time at once.<br>
<br>
So there are two things: a review latency, *and* a merge latency.<br>
<br>
However, two things are also clear:<br>
<br>
 1) This is annoying for people who submit 'rapid improvements', e.g.<br>
in the process of working on GHC, you may fix 4 or 5 things, and then<br>
not having those in the mainline is a bit of a drain!<br>
 2) Phabricator building patches actually means the merge latency can<br>
be *shorter*, because in the past, we'd always have to double check if<br>
a patch worked in the first place (so it took *even longer* before!)<br>
<br>
Another thing is that I'm the primary person who lands things off<br>
Phabricator, although occasionally other people do too. This is<br>
somewhat suboptimal in some cases, since really, providing something<br>
has the OK (from me or someone else), anyone should be able to merge<br>
it. So I think this can be improved too.<br>
<br>
Finally, it's also worth mentioning that Phabricator reviews are<br>
special (and unlike GitHub) in that people who are *not* reviewers *do<br>
not* see the patch by default! That means if I am the *only* person on<br>
the review, it is pretty high guarantee that the review will only be<br>
done by me, and it will only be merged by me, unless I poke someone<br>
else. Others can see your review using a slightly different search<br>
criterion, however, but that's not the default.<br>
<br>
Note this is not a mistake - it is intentional in the design. Why?<br>
Because realistically, I'd say for about 85% of the patches that come<br>
in, they are irrelevant to 90% of all GHC developers, and<br>
historically, 90% of developers will never bother committing it<br>
either. It is often pointless to spam them with emails, and enlarging<br>
their review queue beyond what's necessary makes things even *worse*<br>
for them, since they can't tell what may really deserve their<br>
attention. I do want more people reviewing code actively - but to do<br>
that, there must be a tradeoff - we should try and keep contributor<br>
burden low. Most developers are just our friends after all, including<br>
you - not paid GHC hackers! I don't want to overburden you; we need<br>
you!<br>
<br>
I am one of the exceptions to this: I realistically care and want to<br>
see about 95% of all patches that go into the tree, at least to keep<br>
up to date with what's happening, and also to ensure things get proper<br>
oversight - by, say, adding someone else to a review who I want to<br>
look at it. This is why I'm the common denominator, and a reviewer of<br>
almost every patch (and I think I'm fairly keen on who might care<br>
about what).<br>
<br>
However it's clear that if this is slowing you down we should try to<br>
fix it - we want you to help after all! We already have nearly 40<br>
people with commit rights to GHC, and you've clearly dedicated<br>
yourself to helping. That's fantastic. Perhaps it's time for you to<br>
enter the fray as well so I can get out of your way. :) But I do still<br>
want you to submit code reviews, as everyone else does - it really<br>
does help everyone, and increases a sense of shared ownership, IMO.<br>
<br>
In light of this though, I do think I need to ramp up my merge<br>
frequency. So how does a plan of just trying to merge all outstanding<br>
patches every day sound? This is normally very trivial amounts of time<br>
these days, considering Phabricator tends to catch the most obvious<br>
failures.<br>
<br>
BTW: I merged your pull request on the Win32 repository, so we can<br>
update MinGW - I didn't realize that it was open at all, and in fact I<br>
completely forgot I had permissions to merge things on that<br>
repository! Most of the external library management is normally dealt<br>
with by Herbert or individual maintainers.<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, Oct 29, 2014 at 6:36 AM, Gintautas Miliauskas<br>
<<a href="mailto:gintautas@miliauskas.lt">gintautas@miliauskas.lt</a>> wrote:<br>
> By the way, regarding that repository, could someone merge my pull request?<br>
><br>
> In general, it's a bit frustrating how a lot of the patches in the<br>
> Phabricator queue seem to take a while to get noticed. Don't take it<br>
> personally, I'm just sharing my impressions, but I do feel it's taking away<br>
> some momentum - not good for me & other contributors, and not good for the<br>
> project. I know reviewers are understaffed, maybe consider spreading commit<br>
> rights a bit more widely until the situation improves?<br>
><br>
> On Wed, Oct 29, 2014 at 11:04 AM, Herbert Valerio Riedel<br>
> <<a href="mailto:hvriedel@gmail.com">hvriedel@gmail.com</a>> wrote:<br>
>><br>
>> On 2014-10-29 at 10:59:18 +0100, Phyx wrote:<br>
>><br>
>> [...]<br>
>><br>
>> >> The Win32 package for example, is dreadfully lacking in<br>
>> >> maintainership. While we merge patches, it would be great to see a<br>
>> >> Windows developer spearhead and clean it up<br>
>> ><br>
>> > A while back I was looking at adding some functionality to this<br>
>> > package, but could never figure out which one was actually being<br>
>> > used. I think there are multiple repositories out there.<br>
>><br>
>> I'm not sure which multiple repositories you have seen, but<br>
>><br>
>>   <a href="http://hackage.haskell.org/package/Win32" target="_blank">http://hackage.haskell.org/package/Win32</a><br>
>><br>
>> points quite clearly to<br>
>><br>
>>   <a href="https://github.com/haskell/win32" target="_blank">https://github.com/haskell/win32</a><br>
>><br>
>> and that's the official upstream repository GHC tracks (via a locally<br>
>> mirrored repo at <a href="http://git.haskell.org" target="_blank">git.haskell.org</a>)<br>
>><br>
>> Cheers,<br>
>>   hvr<br>
><br>
><br>
><br>
><br>
> --<br>
> Gintautas Miliauskas<br>
><br>
</div></div><span class="im HOEnZb">> _______________________________________________<br>
> ghc-devs mailing list<br>
> <a href="mailto:ghc-devs@haskell.org">ghc-devs@haskell.org</a><br>
> <a href="http://www.haskell.org/mailman/listinfo/ghc-devs" target="_blank">http://www.haskell.org/mailman/listinfo/ghc-devs</a><br>
><br>
<br>
<br>
<br>
</span><div class="HOEnZb"><div class="h5">--<br>
Regards,<br>
<br>
Austin Seipp, Haskell Consultant<br>
Well-Typed LLP, <a href="http://www.well-typed.com/" target="_blank">http://www.well-typed.com/</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">Gintautas Miliauskas</div>
</div>