I definitely agree with all of these points. With our current setup, the only way a committer can close a PR is by issuing a commit with the magic "This closes ..." clause. The submitter of the PR is the only one who can actually close it in GitHub.
I don't want to hijack the discussion with a different topic, but it might be worth considering switching to the ASF's GitBox integration [1], which I believe lets us use Github as a real repository, rather than just a mirror. It seems like it would make it easier to manage the PRs in the event that we did implement a policy like Mark and Joe described. [1] https://gitbox.apache.org/repos/asf On Mon, Jan 29, 2018 at 11:34 AM, Joe Witt <joe.w...@gmail.com> wrote: > Mark > > Thanks for brining this up. I do agree. > > We need to probably provide more description on the contributor guide > or elsewhere of which aspects makes PRs easier to commit: > - They have unit tests which cover core capabilities but if they're > cloud service dependent or highly network/disk oriented they have > integration tests instead of unit tests for the high risk or > environmentally sensitive bits. > - They have *thoroughly* reviewed and covered License and Notice > updates and are done consistently with the L&N of the rest of the > project. > - They pass all checks on Travis-CI > - If they required manual integration tests that detailed > instructions/explanations of external system setup and configuration > and test processes is provided. > > And maybe some explanation of which items are very difficult to get > good reviewer help on: > - Things which integrate with external systems that are not easily > replicated for testing. Consider whiz-bang database StoreIt. If we > dont have others aware of or famiilar with the StoreIt system it is > really tough to find a good reviewer and timely response. > > We also need to revisit this as we progress an extension registry mechanism. > > Thanks > > On Mon, Jan 29, 2018 at 11:29 AM, Mark Payne <marka...@hotmail.com> wrote: >> All, >> >> We do from time to time go through the backlog of PR's that need to be >> reviewed and >> start a "cleansing" process, closing out any old PR's that appear to have >> stalled out. >> When we do this, though, we typically will start sending out e-mails asking >> if there are >> any stalled PR's that we shouldn't close and start trying to decipher which >> ones are okay >> to close out and which ones are not. This puts quite an onus on the >> committer who is >> trying to clean this up. It also can result in having a large number of >> outstanding Pull Requests, >> which I believe makes the community look bad because it gives the appearance >> that we are >> not doing a good job of being responsive to Pull Requests that are submitted. >> >> I would like to propose that we set a new "standard" that is: if we have any >> Pull Request >> that has been stalled (and by "stalled" I mean a committer has reviewed the >> PR and did >> not merge but asked for clarifications or modifications and the contributor >> has not pushed >> any new commit or responded to the comments) for at least 30 days, that we >> go ahead >> and close the Pull Request (after commenting on the PR that it is being >> closed due to a lack >> of activity and that the contributor is more than welcome to open a new PR >> if necessary). >> >> I feel like this gives contributors enough time to address concerns and it >> is simple enough >> to create a new Pull Request if the need arises. Alternatively, if the >> contributor realizes that >> they need more time, they can simply comment on the PR that they are still >> interested in >> working on it but just need more time, and the simple act of commenting will >> mean that the >> PR is no longer stalled, as defined above. >> >> Any thoughts on such a proposal? Any better alternatives that people have in >> mind? >> >> Thanks >> -Mark
