Mike D., I loved your response, especially for researching what other projects do!
... more responses within... On Tue, Dec 3, 2019 at 2:42 PM Mike Drob <md...@apache.org> wrote: > I'm coming late into this thread because a lot of the discussion happened > while I was offline, so some of the focus has shifted, but I'll try to > address a few topics that were brought up earlier anyway. In an effort to > be brief, I'll try to summarize sentiments rather than addressing specific > quotes - if I mischaracterize something please let me know! > > First, I don't believe that RTC requires consensus voting with 3 reviews > per commit or anything nearly so burdensome. A brief survey of other Apache > projects shows that most on RTC only need a single review, and also can > include exceptions for trivial changes like we are discussing here. If > we're trying to find a compromise position, I personally would prefer to be > more on the RTC side because I think it's a more responsible place to be > for a project that backs billions of dollars of revenue across hundreds of > companies annually. > > Spark is pretty strict RTC, but with such a volume of contributions it may > be the only option sustainable for them. > https://spark.apache.org/contributing.html > HBase requires a review and has an exception for trivial patches - > http://hbase.apache.org/book.html#_review > Yetus requires reviews, but allows binding review from non-committers, and > has a no review expiration. https://s.apache.org/mi0r8 - there's a lot of > other good discussion there too. > Zookeeper requires a minimum one day waiting period on all code change, > but does not require explicit reviews. - > https://zookeeper.apache.org/bylaws.html > > One piece I'll echo from the Yetus discussion is that if we have a habit > of review, then we're less likely to get stagnant patches, and we're also > more likely to engage with non-committer contributions. If we don't have > that habit of reviews, then patches do get stale, and > trust/self-enforcement becomes the only sustainable way forward. > > A second point that I'm also concerned about is the idea that we don't > want JIRA issues or CHANGES entries for trivial or even minor patches. This > has a huge impact on potential contributors and their accessibility to the > project. If contributors aren't credited for all of their work, then it > makes it harder for them to reach invitation as a committer. As a personal > example, a lot of my changes were around logging and error handling, which > are minor on your list but fairly important for a supporter in aggregate. > If the community signalled that the work was less valuable, less visible, > and less likely to be accepted (each of which can follow from the previous > I think) then I don't know that I would have been motivated to try and > contribute those issues. > Our CHANGES.txt tries to simultaneously be a useful document in informing users (and us) of what was changed for what issue that we might actually care about, and *also* give kudos to contributors. There is a lot of noise in there, as it is. If hypothetically a contributor files a JIRA issue with minor/trivial priority, then maybe a git author tag is enough? Or if not then maybe adding a special section in the CHANGES.txt for a special thanks to contributors of unspecified issues? > To the point about security issues, that's something that should probably > be addressed explicitly on the security/private list, and it absolutely > needs review if only so that other developers can learn and avoid those > issues again. That's where the power of community is really important, and > I don't expect issues like that to sit around with a patch waiting for very > long anyway. > > Overall, I think following in the Yetus or ZK model with a 72 hour timeout > is a reasonable compromise, especially because a hard shift from CTR to RTC > would need a corresponding culture shift that may not happen immediately. > Yes I agree. Can you suggest a proposed guideline (or perhaps actual policy? hmm)? Today our guidelines don't quite have this rule, which thus allows a committer to commit a major change immediately without a review (ouch!). Surely we don't *actually* want to allow that. > > Mike >
