Hi Everyone, I hope you all had a lovely holiday period. >
Thank you David and Caleb for your continuous work dealing with the practicalities involved in getting the merge to happen! And thank you Benedict for starting the thread – it is an important topic for us to continue as CEPs and feature branches become a thing! Reading the thread I see some quite distinct perspectives, and it's making it difficult to have as constructive a dialogue as we should wish for. As is usual we see this balanced by the productive collaboration happening but hidden in the jira tickets. I want to raise the following points, not as things to solve necessarily but to help us appreciate just how different our PoVs are and how easy it is for us to be talking past each other. If you immediately disagree with any of them I suggest you read below how my PoV gets me there. 1. Work based on a notion of meritocracy. If you do the work, or have done the work, your input weighs more. 2. Promote teamwork and value an inclusive culture. 3. Where the work is, or has been committed to, can be considered irrelevant (in the context of this thread). 4. There are different types of reviews. We want to embrace them all. 5. We gate on either CI (not only circleci). Do not trash or reduce parity in the other. 6. A feature branch != release branch/trunk. 7. The cep-15-accord was not ready to merge, this thread helped smoke out a number of minor things. 8. Merging of feature branches should be quick and light team-work. Going into these in more detail… 1+3) If a patch from a pmc engineer has been reviewed by two other pmc engineers, CI has been run, and they are about to merge, a new reviewer interjecting late can expect very little to happen pre-merge. If there's something serious it needs to be explained quickly. We value people's time, productiveness, and expertise. This applies everywhere in the project: from tickets with patch files to CEP work and feature branches. 2) The more eyes the better. Everything we do is transparent, and we should always be receptive and by default open to new input regardless of where in the process we are and where it comes from. First be receptive and hear people out, afterwards make the "now, or later" decision. Take the time to explain why a "we'll do it later" decision – it might not be obvious to everyone, it helps newcomers understand the project dynamics, and it sets project precedence. Use (1) as needed. 4) A lot of the thread has focused on reviews of Accord and its technical components. There are other types of reviews. Examples are docs (internal and external), the build system, our CIs and integrations, releases and ASF policies. Henrik pointed out that it is wasteful for these reviews to happen early in the review window. In addition, they are quite small in comparison to initial technical reviews and equally quick to address. The larger the patch the more we can expect others stepping in with these types of tail-end reviews. If we think of the review process as promoting teamwork and an inclusive culture, we can also think of reviews akin to pair-programming that help mentor those less knowledgeable in an area of the code. This needs to be time-boxed ofc, but IMHO we should keep an open mind to it – these types of reviews do surprisingly lead to bug fixes and improvements with the reviewer acting as a rubber-duck. This type of review we would want early, not late. There's also another type of review here. If the author of another CEP has another feature branch about to merge, they may want to review the branch to get ahead of possible incoming conflicts. This is a review that cares nothing about Accord itself. Their evaluation is if there's any project-wide ROI if some changes happen before the merge. Again, teamwork. 5) It's been mentioned "always shippable trunk according to circleci". That's not true: we are always shippable according to *either* CI. There are folk just using ci-cassandra for our pre-commit gateway. It is important that you don't trash the other CI system, particularly when it comes to parity of the tests that we run. If you introduce new tests in one CI it is on you to introduce it to the other CI, and therefore run both CI pre-merge. This does not imply a merge is blocked on green ci-cassandra.a.o 6) It's been mentioned that code committed to a feature branch can be considered finalised, i.e. the review window is closed. I don't buy this. We don't cut releases off feature branches and don't have a policy of "always stable feature-branch". Both (4) and (5) help illustrate this. Thinking about the different types of reviews and concerns we have on trunk was my background to presuming the review window is open until the final atomic commits to our release branches. And our stable trunk agreement and efforts is my presumption that trunk is to be treated like a release branch. This supports (3), it makes no difference if the work could be in a patch or in a feature branch, if we honour (2) and practice (1). 7+8) This would not have happened without Benedict starting this thread, or without Caleb and David. Work is happening in tickets, I'm very grateful for it. We want the final phase to be quick and light team-work. The thread demonstrates the need to have a better process/precedent around this for feature branches. What we have as a bar for committing patches should be re-used, appreciating that the size of the work correlates to the benefits of more eyes and the number of concerns it touches. IMHO we should also keep this discussion evolving over a few feature branch merges, as each may shed light on different challenges. I am keen and will have more time to help next week. It looks like it's down to only a few topics we can solve quickly: docs, CI, submodules, build+release, and whether they are done pre-merge or post-merge we can leave to those doing it. I hope that we can focus on fostering an inclusive culture, balancing it with meritocracy and pragmatism. I'm also up to organising a zoom/meet for folk that want to discuss this further, to help us connect together so many distinct PoVs. In a call together it is much easier. cheers.
