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