Agree with everything being said here. We clearly need to better manage the number of opened PRs and also remind the community that contributing code is great but that helping in the review process will create a virtuous circle and benefit the community.
Pierre 2018-01-29 18:48 GMT+01:00 Joe Witt <[email protected]>: > maybe kick that gitbox thread to a vote since it is decent change to > the workflow. > > Thanks > > On Mon, Jan 29, 2018 at 12:45 PM, Aldrin Piri <[email protected]> > wrote: > > Gitbox was favorably received when we discussed it prior: > > https://lists.apache.org/thread.html/de5e103994f356b1b8a572410938ee > f44af8cb352210e35305c04bc9@%3Cdev.nifi.apache.org%3E > > > > I would be in favor of moving ahead with it and would be happy to get > > things moving if it still seems agreeable. > > > > --aldrin > > > > On Mon, Jan 29, 2018 at 11:46 AM, Bryan Bende <[email protected]> wrote: > > > >> 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 <[email protected]> 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 <[email protected]> > >> 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 > >> >
