> Don't we follow a principle of always shippable trunk? This was actually a > reason why I sidelined the talk about post-merge review, because it implies > that the code wasn't "good enough"/perfect when it was first merged. We follow a principle of "always shippable trunk according to circleci" as of cutting 4.1, which has been adhered to in this case. There's every likelihood that ASF CI will detonate when this is merged in, but it's also already *currently* detonating repeatedly which we're working on so... /shrug
If we block Accord merging on green ASF CI, we'll be in the same boat we were in with 4.1 and never ship until we tear certain things down to the studs and rebuild them. I don't think it's reasonable to put that burden (ASF CI must be green) on *any* ticket right now, much less one that has a potentially high integration cost to the entire project if it stagnates over time. > plenty of experience with situations where even if an engineer, including > myself sometimes, wanted to work on fixing some technical debt, the > employer's project management processes simply wouldn't prioritize that work > and it was left for years Seems like it's a healthy mix of debt and bikeshedding for us historically. The former we don't want to sneak in, the latter, well, that I'm less concerned about personally. :) And I think part of the "two committers +1" bar is about trying to keep our debt low. On Tue, Jan 31, 2023, at 2:45 AM, Benedict wrote: > > Do you mean as part of a blocking review, or just in general? > > I don’t honestly understand the focus on ninja fixes or rebasing, in either > context. Why would a project rebase its work until ready to merge? Why would > you worry that a team well resourced with experienced contributors (30+yrs > between them) don’t know how to work correctly with ninja fixes? These are > all minor details, surely? > > I understand your concern around flaky tests, particularly since it seems > other work has let some slip through. I don’t believe this is a blocking > review concern, as it represents its own blocking requirement. I believe I > have responded positively to this query already though? > > >> On 31 Jan 2023, at 01:46, Ekaterina Dimitrova <e.dimitr...@gmail.com> wrote: >> >> >> Benedict, Is it an issue to ask whether flaky tests will be addressed as per >> our project agreement? Or about ninja fixes and why a branch was not rebased >> during development? What did I miss? >> >> By the way I do not ask these questions to block anyone, even suggested to >> help with CI…it’s a pity this part was dismissed… >> >> I can see that Caleb is handling the things around the merge ticket with >> high attention to the details as always. I can only thank him! >> >> At this point I see this thread mostly as - this is the first feature branch >> since quite some time, people are unsure about certain things - let’s see >> how we handle this for the next time to be more efficient and clear. I >> think you already took some actions in your suggestion earlier today with >> the document around comments. >> >> On Mon, 30 Jan 2023 at 20:16, Benedict <bened...@apache.org> wrote: >>> >>> Review should primarily ask: "is this correct?" and "could this be done >>> differently (clearer, faster, more correct, etc)?" Blocking reviews >>> especially, because why else would a reasonable contributor want to block >>> progress? >>> >>> These questions can of course be asked of implementation details for any >>> CEP. >>> >>> I have said before, a proposal to conduct a blocking review of this kind - >>> if very late in my view - would be valid, though timeline would have to be >>> debated. >>> >>> Reviewers with weaker aspirations have plenty of time available to them - >>> two weeks have already passed, and another couple will likely yet (there >>> isn't a rush). But it is novel to propose that such optional reviews be >>> treated as blocking. >>> >>> >>>> On 30 Jan 2023, at 23:04, Henrik Ingo <henrik.i...@datastax.com> wrote: >>>> >>>> Ooops, I missed copy pasting this reply into my previous email: >>>> >>>> On Fri, Jan 27, 2023 at 11:21 PM Benedict <bened...@apache.org> wrote: >>>>> >>>>>> I'm realizing in retrospect this leaves ambiguity >>>>> >>>>> Another misreading at least of the *intent* of these clauses, is that >>>>> they were to ensure that concerns about a *design and approach* are >>>>> listened to, and addressed to the satisfaction of interested parties. It >>>>> was essentially codifying the project’s long term etiquette around pieces >>>>> of work with either competing proposals or fundamental concerns. It has >>>>> historically helped to avoid escalation to vetoes, or reverting code >>>>> after commit. >>>>> >>>>> It wasn’t intended that *any* reason might be invoked, as seems to have >>>>> been inferred, and perhaps this should be clarified, though I had hoped >>>>> it would be captured by the word “reasonable". Minor concerns that >>>>> haven’t been caught by the initial review process can always be addressed >>>>> in follow-up work, as is very common. >>>>> >>>> >>>> Wouldn't you expect such concerns to at least partially now have been >>>> covered in the CEP discussion, up front? I would expect at most at this >>>> stage someone could validate that the implementation follows the CEP. But >>>> I wouldn't expect a debate on competing approaches at this stage. >>>> >>>> henrik >>>> -- >>>> >>>> >>>> >>>> >>>> *Henrik Ingo* >>>> >>>> *c*. +358 40 569 7354 >>>> >>>> *w*. _www.datastax.com_ >>>> >>>> __ <https://www.facebook.com/datastax> __ <https://twitter.com/datastax> >>>> __ <https://www.linkedin.com/company/datastax/> __ >>>> <https://github.com/datastax/> >>>>
