It seems like one of the main points of contention isn’t so much the the content of the patch, but more about the amount of review this patch has/will receive relative to its perceived risk. If it’s the latter, then I think it would be more effective to explain why that’s the case, and what level of review would be more appropriate.
I’m personally +0 on requiring additional review. I feel like the 3 people involved so far have sufficient expertise, and trust them to be responsible, including soliciting additional reviews if they feel they’re needed. If dev@ does collectively want more eyes on this, I’d suggest we solicit reviews from people who are very familiar with the messaging code, and let them decide what additional work and documentation they’d need to make a review manageable, if any. Everyone has their own review style, and there’s no need to ask for a bunch of additional work if it’s not needed. > On Apr 12, 2019, at 12:46 PM, Jordan West <jorda...@gmail.com> wrote: > > Since their seems to be an assumption that I haven’t read the code let me > clarify: I am working on making time to be a reviewer on this and I have > already spent a few hours with the patch before I sent any replies, likely > more than most who are replying here. Again, because I disagree on > non-technical matters does not mean I haven’t considered the technical. I > am sharing what I think is necessary for the authors > to make review higher quality. I will not compromise my review standards on > a patch like this as I have said already. Telling me to review it to talk > more about it directly ignores my feedback and requires me to acquiesce all > of my concerns, which as I said I won’t do as a reviewer. > > And yes I am arguing for changing how the Cassandra community approaches > large patches. In the same way the freeze changed how we approached major > releases and the decision to do so has been a net benefit as measured by > quality and stability. Existing community members have already chimed in in > support of things like better commit hygiene. > > The past approaches haven’t prioritized quality and stability and it really > shows. What I and others here are suggesting has worked all over our > industry and is adopted by companies big (like google as i linked > previously) and small (like many startups I and others have worked for). > Everything we want to do: better testing, better review, better code, is > made easier with better design review, better discussion, and more > digestible patches among many of the other things suggested in this thread. > > Jordan > > On Fri, Apr 12, 2019 at 12:01 PM Benedict Elliott Smith <bened...@apache.org> > wrote: > >> I would once again exhort everyone making these kinds of comment to >> actually read the code, and to comment on Jira. Preferably with a >> justification by reference to the code for how or why it would improve the >> patch. >> >> As far as a design document is concerned, it’s very unclear what is being >> requested. We already had plans, as Jordan knows, to produce a wiki page >> for posterity, and a blog post closer to release. However, I have never >> heard of this as a requirement for review, or for commit. We have so far >> taken two members of the community through the patch over video chat, and >> would be more than happy to do the same for others. So far nobody has had >> any difficulty getting to grips with its structure. >> >> If the project wants to modify its normal process for putting a patch >> together, this is a whole different can of worms, and I am strongly -1. >> I’m not sure what precedent we’re trying to set imposing arbitrary >> constraints pre-commit for work that has already met the project’s >> inclusion criteria. >> >> >>> On 12 Apr 2019, at 18:58, Pavel Yaskevich <pove...@gmail.com> wrote: >>> >>> I haven't actually looked at the code >> >> >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@cassandra.apache.org For additional commands, e-mail: dev-h...@cassandra.apache.org
