Yes, reviews can and they happen any time and anyone can do them if they have time and interest before/after merge and anyone can raise questions or concerns. It was happening before, it happens now, it will happen in the future. The beauty of open source.
About this merge - I would personally love to see everything rebased and things rerun in CI which no one has a doubt it will happen. Benedict already mentioned also the repeatable jobs will be used, thanks! (I know how disappointing it can be to find a flaky test but it is way easier to fix it while you are still on top of things, instead of, half year away for example.) I am not sure I understand this - “ There exists test failures, but those are due to python-dtest not knowing about the accord table breaking a few tests (blocker for trunk merge, it means trunk dtest must now known about accord), or flaky tests that committers tend to merge anyways (such as specific tests that fail often, known issues, OS allocating of bind address, etc.)” Do you plan to merge before fixing the tests? I am confused I think Henrik brought an interesting point about releasable trunk. I don’t think that we are yet there with the Jenkins issues, but we are getting close and we strive for that, no? My understanding at this point is: 1) we rebase and run CI, get final reviewer approval before a commit. I am not saying it hasn’t happened with the Accord tickets,no, but I am still not sure why the branch was not rebased to be honest. Probably there was some reasoning I am just not familiar with it but I am definitely curious. I find a lot of sense in agreeing in the future to keep feature branches rebased when commits land there. 2) If we encounter new issues/flaky tests/review comments we unfortunately get back to them and we fix them pre-commit on tickets. On the bright side, again, it is way easier to fix them before things got in the mix and time pass. And I believe this is the trunk agreement and the quality bar we try to keep. Also, knowing how you were always adhering strictly to two committers and CI runs pre-commit to the feature branch plus Benedict’s explanation around mainly additions of code, I don’t see any reason for anyone to be worried. I am optimistic we will see very soon things landing in a good shape. 3) what is the story with the ninja fixes? Are they trivial ninja fixes as Josh mentioned or additional small commits? Were they reviewed? Are they going to be squashed? (Take these as honest questions, I just don’t know the plan and I am trying to create my mental model). I will be happy to add a section around ninja commits on our “how to commit page” Unfortunately, I disagree that finding a bug long after something was committed is the same as seeing one before commit. I see people on numerous occasions fixing things and revising. It is frustrating and I can imagine how after that much work around Accord people just want it in. But don’t you want to close the lose ends earlier than later? Yes, you’d say this is long-term committed but I am afraid if you are rebasing and squashing, I still look at the situation in a bit different way. Just my experience with Cassandra where constantly the most unexpected things fail me :-) Interesting points around CI, I am wondering whether we want to run the code in both systems before commit as Jenkins also runs packaging and burn tests. I will be happy to help to push the branch into Jenkins or CircleCI if help is needed. I don’t have time for a review but I don’t mind to help revise the CI results and identify what is old and (hopefully nothing) what is new. Also, when the time approaches, shall we agree on a particular day and ask people not to commit anything for a day or so maybe until you test and commit? To give you the chance to wrap up without interruptions? I think it will be nice to schedule something. BR, Ekaterina On Mon, 30 Jan 2023 at 18:54, David Capwell <dcapw...@apple.com> wrote: > But during development, did you ever run nightly tests / all tests? > > > I strongly recommend people I work with to use my script that modifies > circle ci, which ops us into running all tests; I tend to see people use > this as it makes CI faster, but has this by product as well. > > So yes, all tests should be run before merge. There are examples of > Jenkins only tests that are not run, but again this is due to existing > limitations with Jenkins. > > > On Jan 30, 2023, at 3:33 PM, Henrik Ingo <henrik.i...@datastax.com> wrote: > > On Tue, Jan 31, 2023 at 1:28 AM David Capwell <dcapw...@apple.com> wrote: > >> If the CI coverage isn't 100%, then we should just identify the gaps, and >> focus on that while preparing to merge >> >> >> It has 100% coverage that is done normally for trunk merges; which is a >> pre-commit CI run in Circle OR Jenkins. >> >> > Sure. And thanks. > > But during development, did you ever run nightly tests / all tests? I > wouldn't want the night after merging to trunk to be the first time those > are run. > > 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/> > > >
