Hi Robert, You raise very good points. The commits generated from the recent rebase-and-merge on main are indeed not signed, and not linked to the original PR. That is a real concern.
Therefore, I also think we should forbid rebase-and-merge again. We can always split a large work into many PRs if needed. Thanks, Alex On Wed, Jun 10, 2026 at 11:19 AM Robert Stupp <[email protected]> wrote: > > Hi all, > > Thanks for driving this discussion. > > Looking at the Spark 4 PR after it was merged with "Rebase and merge" [1], > I think the resulting history shows the tradeoff more clearly. > > The individual commits on main are mostly mechanical > copy/move/restore/refactor steps. > What I miss in the commit subjects / linear history [2] is the direct > attribution to the originating pull request, and the GitHub "Verified" > label [3]. > > If I understand correctly, the reason for using rebase-and-merge here was > to preserve file history across copied/moved/refactored Spark files. > That is a reasonable goal, but Git does not record moves or copies as > durable metadata. > It stores snapshots, and commands/tools infer rename/copy history later > using similarity heuristics and command/config options. > That may work today, but future edits, splits, deletions, or refactors can > make the relationship harder to infer, or prevent it from being inferred > reliably. > I raised a similar concern earlier here [4]. > > There is also a supply-chain/audit tradeoff. > The "Verified" label is useful as an authenticity and tamper-evidence > signal. > GitHub documents that with "Rebase and merge," it creates modified commits > and cannot sign them on the contributor's behalf [5]. > So using the GitHub rebase button weakens that signal on main. > > A practical concern regarding accidental use is that GitHub appears to > remember the last merge option selected by a user. > If "Squash and merge" remains enabled alongside either "Rebase and merge" > or "Merge commit," it is easy to accidentally use the wrong option on a > later PR. > > For these reasons, I would prefer to keep only "Squash and merge" enabled > by default. > > If a future change really needs preserved commit boundaries, I think we > should handle that explicitly: either split it into multiple coherent PRs, > or temporarily enable "Rebase and merge" for a clearly justified > exceptional case, then disable it again afterwards. > > Robert > > [1] https://github.com/apache/polaris/pull/4612 > [2] > https://github.com/apache/polaris/commits/0d23d785a3bf282920d0ab17c68151ce9fe904bd/ > [3] > https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification#about-commit-signature-verification > [4] https://github.com/apache/polaris/pull/4535#issuecomment-4563754649 > [5] > https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/about-merge-methods-on-github > > On Tue, Jun 9, 2026 at 5:50 AM Dmitri Bourlatchkov <[email protected]> wrote: > > > Hi Yong, > > > > I believe GH remembers the last merge action "per user". After you do a > > normal squash merge, it should revert the default to squash. > > > > Cheers, > > Dmitri. > > > > On Mon, Jun 8, 2026 at 11:47 PM Yong Zheng <[email protected]> > > wrote: > > > > > Hi, > > > > > > One minor thing I noticed: after using "Rebase and merge" for one PR, > > > GitHub seemed to keep that as the default merge option for a different PR > > > that I was planning to merge. I noticed it before merging and manually > > > switched it back. Is that expected behavior in GitHub? > > > > > > Thanks, > > > Yong > > > > > > On Mon, Jun 8, 2026 at 8:42 PM Yong Zheng <[email protected]> wrote: > > > > > > > Hi, > > > > > > > > So if u did a 'git mv old new', the new file is till been tracked and > > > will > > > > keep the history. However, if u do `git mv old common` then create a > > new > > > > old file (similar to what we are doing in the spark one, only one of > > the > > > > file will have history (in this case the common will have history and > > old > > > > won't have). To save the history on both, we will need to do the move > > > then > > > > commit and reset head of the move file (so the restored file along with > > > > common will share history). > > > > > > > > Then back to this specific example, the trick above will only work if > > we > > > > are doing non-squash merging as if we do a squash merge, the commit > > > history > > > > would be gone again thus only one will have historical. > > > > > > > > Thanks, > > > > Yong > > > > > > > > On 2026/06/08 15:33:50 Dmitri Bourlatchkov wrote: > > > > > Hi All, > > > > > > > > > > Re: PR [4612] - I actually tried squash-merging it into `main` > > locally > > > > and > > > > > line attribution appeared to be preserved on the moved code. Perhaps > > > I'm > > > > > missing something about the history problem in a squashed merge. > > > > > > > > > > In any case, I'm fine with the rebase-and-merge workflow as long as > > it > > > is > > > > > "used wisely". > > > > > > > > > > As for merge commits, I'd prefer to avoid them on `main`. > > > > > > > > > > [4612] https://github.com/apache/polaris/pull/4612 > > > > > > > > > > Cheers, > > > > > Dmitri. > > > > > > > > > > On Mon, Jun 8, 2026 at 10:16 AM Yong Zheng <[email protected]> > > > > wrote: > > > > > > > > > > > Hi Alex, > > > > > > > > > > > > This is mainly for https://github.com/apache/polaris/pull/4612. > > > > > > > > > > > > We started with spark 3.5 and added code for spark 4.0 then later > > > > refactor > > > > > > those into common. Without using what JB created in this PR, the > > > commit > > > > > > history will get squash away. Taking one example, assuming dev A > > > > created > > > > > > the changes for 3.5 earlier and I tried to copy the same code to > > > spark > > > > 4.0. > > > > > > In the commit history, it will show all codes in 4.0 are done by > > me. > > > > And > > > > > > the problem may get worse over time such as spark 3.5 EOL then we > > > > dropped > > > > > > the support for 3.5 completely. In this case the commits made by > > dev > > > A > > > > will > > > > > > be no where to be found from 4.0. > > > > > > > > > > > > Thanks, > > > > > > Yong Zheng > > > > > > > > > > > > > On Jun 8, 2026, at 8:39 AM, Alexandre Dutra <[email protected]> > > > > wrote: > > > > > > > > > > > > > > Hi JB, > > > > > > > > > > > > > > Did you have a specific use case in mind when you opened the PR? > > > > > > > > > > > > > > And out of curiosity, is it possible to keep "rebase and merge" > > > > > > > disabled globally but selectively enabled for some "special" > > > > branches? > > > > > > > > > > > > > > Thanks, > > > > > > > Alex > > > > > > > > > > > > > >> On Mon, Jun 8, 2026 at 12:50 PM Jean-Baptiste Onofré < > > > > [email protected]> > > > > > > wrote: > > > > > > >> > > > > > > >> Let’s wait other feedback. I agree for merge commit: it should > > be > > > > > > disabled. > > > > > > >> However rebase and merge can be informal and I trust the > > > committers > > > > to > > > > > > use > > > > > > >> the best strategy. Generally speaking I always prefer squash and > > > > merge > > > > > > with > > > > > > >> multiple smaller PRs but I also understand that in some case we > > > can > > > > > > prefer > > > > > > >> larger PRs with commits. > > > > > > >> > > > > > > >> Regards > > > > > > >> JB > > > > > > >> > > > > > > >> Le lun. 8 juin 2026 à 11:57, Alexandre Dutra <[email protected] > > > > > > a > > > > > > écrit : > > > > > > >> > > > > > > >>>> If we rebase-and-merge those, all that noise lands on main. > > > > > > >>> > > > > > > >>> That's why I said "when used wisely": this strategy can be > > useful > > > > e.g. > > > > > > >>> if a feature branch, containing many meaningful, independent > > > > commits, > > > > > > >>> is to be rebased on top of main. "Squash and merge" would > > squash > > > > > > >>> everything together which is too coarse-grained. But apart from > > > > this > > > > > > >>> use case, I don't see other useful usages of "rebase and > > merge", > > > > and > > > > > > >>> having it enabled means we need to trust all committers to pick > > > the > > > > > > >>> best merge strategy carefully. > > > > > > >>> > > > > > > >>> Thanks, > > > > > > >>> Alex > > > > > > >>> > > > > > > >>> On Mon, Jun 8, 2026 at 11:45 AM Robert Stupp <[email protected]> > > > > wrote: > > > > > > >>>> > > > > > > >>>> Hi, > > > > > > >>>> > > > > > > >>>> My main concern with rebase-and-merge is that many PRs > > naturally > > > > > > collect > > > > > > >>>> random cleanup commits during review: fixing formatting, > > > > addressing > > > > > > one > > > > > > >>>> comment, fixing CI, another small tweak, etc. > > > > > > >>>> > > > > > > >>>> If we rebase-and-merge those, all that noise lands on main. > > > > > > >>>> I don’t think that makes the history better. > > > > > > >>>> Usually the useful unit is the PR, not every intermediate > > commit > > > > > > someone > > > > > > >>>> pushed while getting it ready. > > > > > > >>>> > > > > > > >>>> So I’m fine with disabling merge commits, but I’d probably > > also > > > > keep > > > > > > >>> rebase > > > > > > >>>> disabled unless we have a clear rule that it’s only used for > > PRs > > > > with > > > > > > a > > > > > > >>>> clean, intentional commit series. > > > > > > >>>> > > > > > > >>>> Robert > > > > > > >>>> > > > > > > >>>> On Mon, Jun 8, 2026 at 11:16 AM Jean-Baptiste Onofré < > > > > [email protected] > > > > > > > > > > > > > >>>> wrote: > > > > > > >>>> > > > > > > >>>>> We can keep our boy squash and merge and rebase and merge, > > and > > > > > > disable > > > > > > >>>>> merge commit. > > > > > > >>>>> > > > > > > >>>>> Le lun. 8 juin 2026 à 09:47, Alexandre Dutra < > > > [email protected]> > > > > a > > > > > > >>> écrit : > > > > > > >>>>> > > > > > > >>>>>> Hi JB. > > > > > > >>>>>> > > > > > > >>>>>> While I see *some* value in the "rebabse & merge" strategy > > > when > > > > > > used > > > > > > >>>>>> wisely, I can hardly imagine a situation where "merge > > commit" > > > is > > > > > > >>>>>> useful. OTOH, it would be really bad if someone used "merge > > > > commit" > > > > > > >>> to > > > > > > >>>>>> merge a PR onto main. > > > > > > >>>>>> > > > > > > >>>>>> So I'm globally -0 on the idea. > > > > > > >>>>>> > > > > > > >>>>>> Thanks, > > > > > > >>>>>> Alex > > > > > > >>>>>> > > > > > > >>>>>> On Mon, Jun 8, 2026 at 7:51 AM Jean-Baptiste Onofré < > > > > > > [email protected] > > > > > > >>>> > > > > > > >>>>>> wrote: > > > > > > >>>>>>> > > > > > > >>>>>>> Hi Yong > > > > > > >>>>>>> > > > > > > >>>>>>> No worries :) I don't see issue with that. We will revert > > if > > > > > > >>> someone > > > > > > >>>>> has > > > > > > >>>>>> a > > > > > > >>>>>>> strong concern (I don't see why as the previous behavior is > > > > still > > > > > > >>>>>>> available). > > > > > > >>>>>>> > > > > > > >>>>>>> Regards > > > > > > >>>>>>> JB > > > > > > >>>>>>> > > > > > > >>>>>>> On Sun, Jun 7, 2026 at 8:25 PM Yong Zheng < > > [email protected] > > > > > > > > > > >>> wrote: > > > > > > >>>>>>> > > > > > > >>>>>>>> +1 > > > > > > >>>>>>>> > > > > > > >>>>>>>> Sorry, I didn't saw this ML and merged the PR already. We > > > can > > > > > > >>> revert > > > > > > >>>>>> that > > > > > > >>>>>>>> in case preferred. > > > > > > >>>>>>>> > > > > > > >>>>>>>> Thanks, > > > > > > >>>>>>>> Yong > > > > > > >>>>>>>> > > > > > > >>>>>>>> On 2026/06/07 16:56:54 Jean-Baptiste Onofré wrote: > > > > > > >>>>>>>>> Hi folks, > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> I created the PR [1] to enable three actions on the > > GitHub > > > > > > >>> Merge > > > > > > >>>>>> button: > > > > > > >>>>>>>>> - Squash and merge (the only action we can do today) > > > > > > >>>>>>>>> - Create a merge commit (preserving the history/commits) > > > > > > >>>>>>>>> - Rebase and merge (preserving the history/commits) > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> You can find more information about what it means here: > > > > > > >>>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>> > > > > > > > > > > > > > > > https://docs.github.com/fr/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> [1] https://github.com/apache/polaris/pull/4634 > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Concretely, once this PR is merged, the merge button will > > > > have > > > > > > >>> a > > > > > > >>>>>> dropdown > > > > > > >>>>>>>>> menu to select the desired action. > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Thoughts? > > > > > > >>>>>>>>> > > > > > > >>>>>>>>> Regards > > > > > > >>>>>>>>> JB > > > > > > >>>>>>>>> > > > > > > >>>>>>>> > > > > > > >>>>>> > > > > > > >>>>> > > > > > > >>> > > > > > > > > > > > > > > > > > > > >
