OK,: fair and good points. It's what I said in my previous email: instead of multiple commits in one PR, better to use smaller PR with squash-and-merge.
Here's the PR to revert to the original state: https://github.com/apache/polaris/pull/4681 Regards JB On Wed, Jun 10, 2026 at 2:04 PM Alexandre Dutra <[email protected]> wrote: > 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 > > > > > > > >>>>>>>>> > > > > > > > >>>>>>>> > > > > > > > >>>>>> > > > > > > > >>>>> > > > > > > > >>> > > > > > > > > > > > > > > > > > > > > > > > > > >
