Hi David, Setting the repo config to always be PR title & body would be good as it's a lot more predictable (most PRs have more than one commit by the end of the PR review, but there are exceptions and it's harder for a committer to update the commit message in such cases).
The reviewers verification makes sense. It would also be good to do something about the "Committer Checklist" bit - it would be annoying to have that in every commit message. We can either remove it from the template (if we think it doesn't add value), automatically remove it during the merge queue job or have a status check for it (along with the reviewers bit). Ismael On Tue, Feb 4, 2025 at 6:55 AM David Arthur <mum...@gmail.com> wrote: > Ismael, thanks for taking a look! > > We would need to alter the default commit message setting for the > repository. The default (which we currently use) is PR title + PR body if > there are multiple commits, or the commit subject and body if it's a single > commit PR. We can change this to always be the PR title and PR body. > > Additionally, we can add a status check that validates that the PR body has > the expected "Reviewers" field before merging. It might also be possible to > automatically set that field. > > I have a feeling it might be possible to alter the commit message in the > merge queue job, but that will take some experimentation. > > -David A > > On Tue, Feb 4, 2025 at 5:09 AM Ismael Juma <m...@ismaeljuma.com> wrote: > > > This looks good to me. > > > > My only comment is that merge queues have a weird limitation: you cannot > > edit the commit message (unlike the traditional flow). I worry that this > > will result in lower quality commit messages. Is there a way to configure > > where the commit message comes from (PR title/description versus > > aggregating all the commit messages in the PR)? > > > > Ismael > > > > On Mon, Feb 3, 2025, 8:02 AM David Arthur <mum...@gmail.com> wrote: > > > > > Happy Monday, all! > > > > > > I'd like to close out this discussion at some point this week, so I > > wanted > > > to bump this up once before moving to a vote. > > > > > > Did anyone have other feedback or concerns about moving towards the > > GitHub > > > Merge Queue? > > > > > > Thanks! > > > David A > > > > > > On Thu, Jan 30, 2025 at 11:11 AM David Arthur <mum...@gmail.com> > wrote: > > > > > > > Uchechi, > > > > > > > > While the build queue can run the builds concurrently, each build is > > > > cumulative. > > > > > > > > For example, A, B, and C are added to the queue and A causes a > problem > > > for > > > > C. > > > > > > > > Merge Queue: A, B, C > > > > > > > > Suppose we have concurrent builds and batching turned on. We might > end > > up > > > > with two builds for these three PRs: > > > > > > > > Build 1: trunk + A > > > > Build 2: trunk + A + B + C > > > > > > > > Each build is going to be cumulative. The point of the concurrency is > > so > > > > we don't have to wait for an in-progress build to finish before we > > start > > > > validating new PRs from the queue. In this case, build 2 would fail > and > > > PRs > > > > B and C would get kicked back to the authors. > > > > > > > > As you can see, the batching and concurrency can complicate matters a > > bit > > > > which is why I suggest we start with no batching or concurrency. > > > > > > > > Hope this helps! > > > > David A > > > > > > > > On Wed, Jan 29, 2025 at 2:34 AM Ukpa Uchechi <ukpauchec...@gmail.com > > > > > > wrote: > > > > > > > >> Hi David, > > > >> > > > >> Thanks for this and for your work improving the Kafka CI. > > > >> > > > >> I have a question: > > > >> > > > >> A problem was mentioned about two PRs acting against each other when > > > >> there’s no delay and being merged concurrently. Wouldn’t this still > > > happen > > > >> in the queue with concurrent merging as well? > > > >> Best Regards, > > > >> Uchechi. > > > >> > > > >> On Fri, 24 Jan 2025 at 03:35, David Arthur <mum...@gmail.com> > wrote: > > > >> > > > >> > Greetings, Kafka community! > > > >> > > > > >> > At long last, the GitHub Merge Queue is upon us. This is a feature > > > that > > > >> > many of us have wanted for quite a while. After many months of > > > >> discussion, > > > >> > the excellent ASF Infra team has delivered! > > > >> > > > > >> > Since this is quite a significant change, I have written up the > > > details > > > >> as > > > >> > a KIP. > > > >> > > > > >> > > > > >> > > > > >> > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1126%3A+Serialize+changes+to+Kafka+with+build+queue > > > >> > > > > >> > Please let me know what you think. > > > >> > > > > >> > -- > > > >> > David Arthur > > > >> > > > > >> > > > > > > > > > > > > -- > > > > David Arthur > > > > > > > > > > > > > -- > > > David Arthur > > > > > > > > -- > David Arthur >
