Re: [PATCH 00/12] RFC: In-core git merge-tree ("Server side merges")
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Jan 26, 2022, 14:44 UTC
- Message-ID
- <CAP8UFD2qFmZ2Adk71SQw9xtq5keZ=d2hcMwF=fs9OtW4==0ZYg@mail.gmail.com>
- In-Reply-To
- <nycvar.QRO.7.76.6.2201261250380.6842@tvgsbejvaqbjf.bet>
Hi Dscho,
On Wed, Jan 26, 2022 at 1:03 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 23 quoted lines
> > Hi Christian, > > On Wed, 26 Jan 2022, Christian Couder wrote: > > > On Sat, Jan 22, 2022 at 10:56 PM Elijah Newren via GitGitGadget > > <gitgitgadget@gmail.com> wrote: > > > > > * Accept an optional --trivial-merge option to get old style merge-tree > > > behavior > > > * Allow both --write-tree and --trivial-merge to be omitted since we can > > > deduce which from number of arguments > > > > I still think that it might be simpler and cleaner to leave 'git > > merge-tree' alone for now, and just add a new command named for > > example 'git write-merge-tree'. > > That would assume that the original `git merge-tree` implementation was > useful. That notion has been thoroughly refuted in the meantime, though. > > I am really opposed to introducing a new command here. Elijah took the > best approach we can take here: save the `merge-tree` command by teaching > it to do something useful.
I think it's a question of point of view. If a command is completely useless, then most of the time it needs to die, not be "saved". We would need good statistics, but I doubt we have "saved" many useless commands before, compared to commands we have just killed.
Show 8 quoted lines
> > Later we can always add flags to 'git merge-tree' or add 'git > > trivial-merge-tree' as an alias for 'git merge-tree', and eventually > > slowly switch 'git merge-tree' to mean only 'git write-merge-tree' if > > that's where we want to go. > > I suggested before, and seem to need to repeat again, that we need to let > ourselves be guided less by hypothetical scenarios, and more by actual, > concrete use cases where the revamped `merge-tree` command is useful.
Ok, see below.
Show 29 quoted lines
> And since I already provided some feedback based on my work from working > on a server-side backend, I am fairly certain that we already have a > pretty good idea where we want to go. > > > > Stuff intentionally NOT included, but which others seemed to feel strongly > > > about; they'd need to convince me more on these: > > > > > > * Any form of diff output[1] > > > > It's not a big issue for me to not include them right now as long as > > it's possible to add cli options later that add them. > > But why? That _so_ smells like a hypothetical scenario. > > We do not need the diffs. It is highly unlikely that the server-side wants > to have diffs, and if a user does want the diffs, it is very, very easy to > generate them by chaining low-level commands. > > So there is absolutely no need for `git merge-tree` to produce diffs. > > > The reason is that I think in many cases when there are conflicts, the > > conflicts will be small and the user will want to see them. So it would > > be simpler to just have an option to show any conflict right away, > > rather than have the user launch another command (a diff-tree against > > which tree and with which options?). > > That assumes that server-side merge UIs will present merge conflicts in > the form of diffs containing merge conflict markers. Which I don't think > will happen, like, ever.
Please take a look at:
https://docs.gitlab.com/ee/user/project/merge_requests/conflicts.html#resolve-conflicts-in-the-inline-editor
As you can see in the image there are conflict markers in the file displayed by the server UI.
Show 9 quoted lines
> In short: I completely disagree that we should introduce a new command, > and I also completely disagree that the `merge-tree` command should output > any diffs. > > I do agree that we need to be mindful of what we actually need, and in > that regard, I reiterate that we need to let concrete use cases guide us. > As part of GitLab, you might be in an excellent position to look at > GitLab's concrete server-side needs when it comes to use `git merge-tree` > to perform merges.
I hope I provided a concrete use case with the link above.