Elijah Newren <new...@gmail.com> writes: > Thanks for continuing to push on this. This looks good so far (to > me), but I was also hoping to see the analogy between these messages > and "Auto-merging $FILE" for regular files mentioned. Both Junio[1] > and I[2] pointed out this similarity, and I think this > similarity/analogy is useful additional motivation for making this > change.
... meaning that it should be discussed and named as the primary reason why this change is a good idea? Re-reading what Leif wrote in the first paragraph, I tend to think that "the more recent version may break us" Leif gives is not a particularly convincing one. After all, if we did not change the commit bound at a submodule since we forked, while they changed it to something else (either old or new), even though our changes may have been fully tested with the version of the submodule we have been testing with, it may break with the version the merged branch has been using. Such an update is cleanly and silently resolved at the tree-level three-way merge, but the risk of breakage is no different to the case this patch adds new notices to. More importantly, the same "the changes we made may get broken by changes in areas that are textually unrelated they made" will happen without submodules. Content-level three-way merges that resolves cleanly at the textual level may need to get semantic adjustment. Do we treat clean 3-way content merges as suspicious and give a similar warning? That smells like madness. But as you said, we give "Auto-merging $FILE" notice to clean 3-way merge at the content-level for normal files, and there is no good reason why we should not do the same for submodules when one fast-forwards to the other, which is an analogue to the content-level 3-way merge where one branch's version is a superset of the other ones. And that is quite a convincing reason why a new "Auto-merging $SUBMODULE" notice is a good idea. > ... > Also, by analogy to the "Auto-merging $FILE" comparison, the "to %s" > on these two lines feels out of place. Users can just look at the > submodule to see what it was updated to. In a sea of output from > merging, this extra detail feels like noise for the standard use-case, > unless I'm misunderstanding how submodules are special. Now you meantion it, that part of the message does look more like a debugging aid than a feature that helps actual end-users. After all, if our side did not change the commit recorded for the submodule while their side changed, we do not report the result of such a tree-level three-way merge that takes what commit they had at their tip.
