Hi! I have contributed a patch or two to GHC, so I guess I’m a reasonable example of an newbie.
The step of nominating reviewers just wouldn’t work for me. I have no idea of who in this project would be willing and able to give a review. Or who the eligible reviewers are. Maybe I’d select someone who haven’t been active for years. If you do this, can you please add an alternative “I’m a clueless newbie, help me select reviewers” to that step? Regards, Niklas > 8 nov. 2019 kl. 11:53 skrev Simon Peyton Jones via ghc-devs > <ghc-devs@haskell.org>: > > | If the maintainers are not willing to either review or find reviewers > | for a new contributors patch > | then it doesn't seem to me that a project wants or values new > | contributors. > > Yes, that would be an unfortunate -- and indeed wrong -- impression to > convey. Thanks for highlighting it. > > You'd like the maintainers to have an *obligation* to cause someone to > produce a good review on every patch. Here's the worst-case scenario: a > well-meaning but inexperienced person produces a stream of large, > ill-thought-out, and mostly wrong patches. To give a guarantee of high > quality reviews of those patches amounts to a blank cheque on the time of > volunteers working mostly in their spare time. > > Now, of course, that's an extreme scenario. But that's why I'm keen to avoid > making it an unconditional obligation that the few maintainers must discharge. > > I don’t think there is really a difference of opinion here. Of course we > welcome patches; of course everyone will try to help find reviewers if they > are lacking! > > So how about this > - the author nominates reviewers > - if he or she finds difficulty in doing so, or the reviewers s/he > nominates are unresponsive, then he or she should ask for help > - maintainers should make efforts to help > > In other words, as an author you remain in control. But help is available if > you need it. > > What do others think? > > Simon > > | -----Original Message----- > | From: Matthew Pickering <matthewtpicker...@gmail.com> > | Sent: 08 November 2019 10:25 > | To: Simon Peyton Jones <simo...@microsoft.com> > | Cc: Ben Gamari <b...@well-typed.com>; ghc-devs@haskell.org > | Subject: Re: Proposed changes to merge request workflow > | > | If the maintainers are not willing to either review or find reviewers > | for a new contributors patch > | then it doesn't seem to me that a project wants or values new > | contributors. > | > | A maintainer can make a value judgement about a patch that is isn't > | worth reviewing, but such > | situations are exceedingly rare. Everyone contributes patches in good > | faith in order to make the compiler better. > | > | Realistically it's impossible to be a good reviewer without having > | implemented patches on the code base. If you don't > | have a good handle for how things work then it's too big to get a feel > | for just by reading the code. You need to learn how things > | fit together by getting stuck writing patches. > | > | At least some of the maintainers are paid to maintain GHC and as such, > | should be expected to perform responsibilities that > | volunteers are not willing to perform. One of these tasks should be > | finding reviewers for all patches and making sure contributions > | do not languish indefinitely. > | > | Apart from this one point the suggested process sounds good but it > | seems to have stalled in the last month. > | > | Cheers, > | > | Matt > | > | On Wed, Oct 9, 2019 at 11:31 AM Simon Peyton Jones > | <simo...@microsoft.com> wrote: > | > > | > | > Make it clear that it is the contributor's responsibility to > | identify > | > | reviewers for their merge requests. > | > | > | > | Asking for reviews is one of the most frustrating parts of > | > | contributing patches, even if you know who to ask! So I think the > | > | maintainer's should be responsible for finding suitable and willing > | > | reviewers. > | > > | > It is true that it's hard to find reviewers. But if it's hard for the > | author it is also hard for the maintainers. A patch is a service that an > | author is offering, which is great. But every patch is owed, as a matter > | of right, suitable and willing reviewers, the patch is /also/ a blank > | cheque that any author can write, but it's up to someone else to pay. > | That's not good either. No author has an unlimited call on the time of > | other volunteers, and I don't think any author truly expects that. > | > > | > It's an informal gift economy. I review your patches (a) because I have > | learned that you have good judgement and write good code (b) because I > | want the bug that you are fixing to be fixed and (c) because you give me > | all sorts of helpful feedback about my patches, or otherwise contribute to > | the community in constructive ways. > | > > | > That may make it hard for /new/ authors to get started. Being an > | assiduous reviewer is an excellent plan, because it gets you into GHC's > | code base, guided by someone else's work; and it earns you all those good- > | contributor points. But even then it may be hard. So I think it's > | absolutely reasonable for authors to ask for help in finding reviewers. > | > > | > But simply saying that it's "the maintainers" responsibility to find > | reviewers goes much too far in the other direction, IMHO. > | > > | > Perhaps we should articulate some of this thinking. > | > > | > Simon > | > > | > | -----Original Message----- > | > | From: ghc-devs <ghc-devs-boun...@haskell.org> On Behalf Of Matthew > | > | Pickering > | > | Sent: 09 October 2019 11:18 > | > | To: Ben Gamari <b...@well-typed.com> > | > | Cc: ghc-devs@haskell.org > | > | Subject: Re: Proposed changes to merge request workflow > | > | > | > | Sounds good in principal but I object to > | > | > | > | > Make it clear that it is the contributor's responsibility to > | identify > | > | reviewers for their merge requests. > | > | > | > | Asking for reviews is one of the most frustrating parts of > | > | contributing patches, even if you know who to ask! So I think the > | > | maintainer's should be responsible for finding suitable and willing > | > | reviewers. > | > | > | > | Cheers, > | > | > | > | Matt > | > | > | > | On Tue, Oct 8, 2019 at 7:17 PM Ben Gamari <b...@well-typed.com> wrote: > | > | > > | > | > tl;dr. I would like feedback on a few proposed changes [1] to our > | merge > | > | > request workflow. > | > | > > | > | > > | > | > Hello everyone, > | > | > > | > | > Over the past six months I have been monitoring the operation of > | our > | > | > merge request workflow, which arose rather organically in the wake > | of > | > | > the initial move to GitLab. While it works reasonably well, there > | is > | > | > clearly room for improvement: > | > | > > | > | > * we have no formal way to track the status of in-flight merge > | > | > requests (e.g. for authors to mark an MR as ready for review or > | > | > reviewers to mark work as ready for merge) > | > | > > | > | > * merge requests still at times languish without review > | > | > > | > | > * the backport protocol is somewhat error prone and requires a > | great > | > | > deal of attention to ensure that patches don't slip through the > | > | > cracks > | > | > > | > | > * there is no technical mechanism to prevent that under-reviewed > | > | > patches from being merged (either intentionally or otherwise) > | to > | > | > `master` > | > | > > | > | > To address this I propose [1] a few changes to our workflow: > | > | > > | > | > 1. Define explicit phases of the merge request lifecycle, > | > | > systematically identified with labels. This will help to make > | it > | > | > clear who is responsible for a merge request at every stage of > | its > | > | > lifecycle. > | > | > > | > | > 2. Make it clear that it is the contributor's responsibility to > | > | > identify reviewers for their merge requests. > | > | > > | > | > 3. Institute a final pre-merge sanity check to ensure that > | > | > patches are adequately reviewed, documented, tested, and have > | had > | > | > their ticket and MR metadata updated. > | > | > > | > | > Note that this is merely a proposal; I am actively seeking input > | from > | > | > the developer community. Do let me know what you think. > | > | > > | > | > Cheers, > | > | > > | > | > - Ben > | > | > > | > | > > | > | > [1] > | > | > | https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.h > | > | askell.org%2Fghc%2Fghc%2Fwikis%2Fproposals%2Fmerge-request- > | > | > | workflow&data=02%7C01%7Csimonpj%40microsoft.com%7Cd1199fd308b442cf744f > | > | > | 08d74ca2074b%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6370621311033130 > | > | > | 52&sdata=SxBADAuF%2FvGzduaytetUzIxGr8lC%2BjTX2eCLNEoOCkQ%3D&reserv > | > | ed=0 > | > | > _______________________________________________ > | > | > ghc-devs mailing list > | > | > ghc-devs@haskell.org > | > | > > | > | > | https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.hask > | > | ell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc- > | > | > | devs&data=02%7C01%7Csimonpj%40microsoft.com%7Cd1199fd308b442cf744f08d7 > | > | > | 4ca2074b%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637062131103313052&a > | > | > | mp;sdata=T%2FyLoRH9BTIVPxMzF0%2BAa3c20qCBkhvQrp53FtROz40%3D&reserved=0 > | > | _______________________________________________ > | > | ghc-devs mailing list > | > | ghc-devs@haskell.org > | > | > | https://nam06.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmail.hask > | > | ell.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fghc- > | > | > | devs&data=02%7C01%7Csimonpj%40microsoft.com%7Cd1199fd308b442cf744f08d7 > | > | > | 4ca2074b%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637062131103323047&a > | > | > | mp;sdata=IwsIP3P6W5qtsLxfePbYOWTXdPLttNMLHWXkuTtVWgI%3D&reserved=0 > _______________________________________________ > ghc-devs mailing list > ghc-devs@haskell.org > http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs _______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
