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 <[email protected]> wrote: > 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. > > On Wed, Oct 29, 2014 at 6:36 AM, Gintautas Miliauskas > <[email protected]> wrote: > > 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 > 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 > > <[email protected]> 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 > > [email protected] > > http://www.haskell.org/mailman/listinfo/ghc-devs > > > > > > -- > Regards, > > Austin Seipp, Haskell Consultant > Well-Typed LLP, http://www.well-typed.com/ > -- Gintautas Miliauskas
_______________________________________________ ghc-devs mailing list [email protected] http://www.haskell.org/mailman/listinfo/ghc-devs
