On Tue, Feb 18, 2025 at 10:55 AM Lucas Oshiro <lucasseikioshiro@xxxxxxxxx> wrote: > > > It may be worth referencing the commit(s) that introduced the behavior > > for other reviewers: commit 68d03e4a6e44 (Implement automatic > > fast-forward merge for submodules, 2010-07-07). > > Ok! I'll inspect the codebase and reference it in a future v2. > > > "referred by it" is hard for me to parse. Maybe something like > > > > """ > > In the case where the path is a submodule, if the HEAD version of the > > submodule is a descendant of the MERGE_HEAD version of the submodule, > > or vice-versa, Git... > > """ > > ? > > Perfect! Actually, I find submodules a little abstract to be explained > using only words, but your sentence is very clear. > > > Also, the references to HEAD and MERGE_HEAD do tie this documentation > > rather directly to `git merge`; the basic idea is applicable to all > > callers of the merge machinery, but none of the other callers use > > MERGE_HEAD (some use CHERRY_PICK_HEAD or REBASE_HEAD), and some do not > > assume HEAD points to one of the parents either (e.g. merge-tree and > > replay). So, if we want to move this somewhere more general, we'd > > need to reword it a bit. > > Given your previous suggestion, what about: > > """ > In the case where the path is a submodule, if one of the versions of > submodule is descendant of another, Git... > """ > > ? That seems like the right direction, but I think "descendant of another" is vague/confusing. Perhaps """ In the case where the path is a submodule, if the submodule commit used on one side of the merge is a descendant of the submodule commit used on the other side of the merge, Git... """ ? > > > Oh, maybe we could put this information in > > Documentation/merge-strategies.txt? Hmm.... > > Looks like a good place to put this. My only concerns are: > > 1. It would need to be documented in both `ort` and `recursive`. I don't > think it would be a big deal as most of the first paragraph of both > strategies are equal. Yes, until `recursive` is deleted anyway. (At which point we'll just remap `recursive` to mean `ort` and not have to have separate documentation for the two.) > 2. Maybe it would detail too much on this specific case, while not > covering others (e.g. changing file permissions, symlinks, etc). Yeah, but we don't have a way to resolve differences for those kinds of changes when neither side matches the base version (unless something like -Xours or -Xtheirs is passed, but even then that belongs under the -X documentation); submodules are somewhat special in that regard. > > Thanks for sending this in. It's always helpful to get researched > > documentation improvements, even if I can't help but nitpick and > > complicate matters here and there.... ;-) > > Thank you! Given how deeply you understand the merge machinery any nitpick > is immensely valuable!
