Hi everyone, I am excited to see this work merged. I noticed the branch is 395 commits behind trunk or not rebased since September last year. I think if Mick or anyone else wants to make a final pass after rebase happens and CI runs - this work can only benefit of that. Squash, rebase and full CI run green is the minimum that, if I read correctly the thread, we all agree on that part. I would say that CI and final check after a long rebase of a patch is a thing we actually do all the time even for small patches when we get back to our backlog of old patches. This doesn’t mean that the previous reviews are dismissed or people not trusted or anything like that. But considering the size and the importance of this work, I can really see only benefit of a final cross-check. As Henrik mentioned me, I am not sure I will have the chance to review this work any time soon (just setting the right expectations up front) but I see at least Mick already mentioning he would do it if there are no other volunteers. Now, whether it will be separate ticket or not, that is a different story. Aren’t the Accord tickets in an epic under which we can document the final rebase, CI runs, etc?
On Tue, 24 Jan 2023 at 9:40, Henrik Ingo <henrik.i...@datastax.com> wrote: > When was the last time the feature branch was rebased? Assuming it's a > while back and the delta is significant, surely it's normal process to > first rebase, run tests, and then present the branch for review? > > To answer your question: The effect of the rebase is then either baked > into the original commits (which I personally dislike), or you can also > have the rebase-induced changes as their own commits. (Which can get > tedious, but has the benefit of making explicit what was only a change due > to rebasing.) Depending on which approach you take when rebasing, a > reviewer would then review accordingly. > > henrik > > On Tue, Jan 24, 2023 at 11:14 AM Benedict <bened...@apache.org> wrote: > >> No, that is not the normal process. What is it you think you would be >> reviewing? There are no diffs produced as part of rebasing, and the purpose >> of review is to ensure code meets out standards, not that the committer is >> competent at rebasing or squashing. Nor are you familiar with the code as >> it was originally reviewed, so would have nothing to compare against. We >> expect a clean CI run, ordinarily, not an additional round of review. If we >> were to expect that, it would be by the original reviewer, not a third >> party, as they are the only ones able to judge the rebase efficiently. >> >> I would support investigating tooling to support reviewing rebases. I’m >> sure such tools and processes exist. But, we don’t have them today and it >> is not a normal part of the review process. If you want to modify, clarify >> or otherwise stipulate new standards or processes, I suggest a separate >> thread. >> >> > How will the existing tickets make it clear when and where their final >> merge happened? >> >> By setting the release version and source control fields. >> >> >> >> On 24 Jan 2023, at 08:43, Mick Semb Wever <m...@apache.org> wrote: >> >> >> >> .... But it's not merge-than-review, because they've already been >>> reviewed, before being merged to the feature branch, by committers >>> (actually PMC members)? >>> >>> You want code that's been written by one PMC member and reviewed by 2 >>> other PMC members to be put up for review by some random 4th party? For how >>> long? >>> >> >> >> It is my hope that the work as-is is not being merged. That there is a >> rebase and some trivial squashing to do. That deserves a quick check by >> another. Ideally this would be one of the existing reviewers (but like any >> other review step, no matter how short and trivial it is, that's still an >> open process). I see others already doing this when rebasing larger patches >> before the final merge. >> >> Will the branch be rebased and cleaned up? >> How will the existing tickets make it clear when and where their final >> merge happened? >> >> >> > > -- > > 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/> > >
