Re: [PATCH 2/2] pull --rebase: reinstate helpful message on abort
Johannes Schindelin writes: > Hi Junio, > > On 2015-10-09 20:36, Junio C Hamano wrote: >> Johannes Schindelin writes: >> >>> When calling `git pull --rebase`, things can go wrong. In such a case, >>> we want to tell the user about the most common ways out of this fix: >>> ... >>> builtin/am.c | 1 + >>> 1 file changed, 1 insertion(+) >> >> It is strange to see a patch to am that does not talk anything about >> it, though. And looking at the codepath, the issue does not have >> much to do with "pull --rebase". It doesn't even have much to do >> with "rebase". This is purely about "am -3" fallback codepath. > > I made it a habit of describing the big picture in commit messages, > including the original motivation for the patch. Naturally, it is > purely an implementation detail that the bug displayed by `git pull > --rebase` is fixed by modifying `am.c`. Yup, but that is "I happened to notice that bug first in a command that uses another command that happens to use this buggy one". That may be interesting in the "peeing in the snow" sense, but not very interesting in the big picture of ensuring the health of the entire codebase. The "common ways out of this" helpful message is not even coming from "pull --rebase" or even "rebase" in the first place. What you are reinstating helpful message on abort is "am -3". "This fixes am -3, hence incidentally fixes rebase and hence fixes pull --rebase, too" would be the most useful way to describe this change. The initial report being about "pull --rebase" is of much lessor importance, I would think. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pull --rebase: reinstate helpful message on abort
Hi Junio, On 2015-10-09 20:36, Junio C Hamano wrote: > Johannes Schindelin writes: > >> When calling `git pull --rebase`, things can go wrong. In such a case, >> we want to tell the user about the most common ways out of this fix: >> ... >> builtin/am.c | 1 + >> 1 file changed, 1 insertion(+) > > It is strange to see a patch to am that does not talk anything about > it, though. And looking at the codepath, the issue does not have > much to do with "pull --rebase". It doesn't even have much to do > with "rebase". This is purely about "am -3" fallback codepath. I made it a habit of describing the big picture in commit messages, including the original motivation for the patch. Naturally, it is purely an implementation detail that the bug displayed by `git pull --rebase` is fixed by modifying `am.c`. > Because fall-back-threeway wants to react to an error (i.e. calls > merge_recursive_generic() and wants to use its return value), but > merge_recursive_generic() can die, it fails to do so. It would not > even run rerere(), for example. Precisely, So the symptom triggering the bug fix was seemingly unrelated to the patch, hence the need for the comprehensive commit message. Since our tastes seem to differ a bit, maybe we can have the best of both worlds by appending the following paragraph to the commit message? -- snipsnap -- This patch actually fixes a deeper-seated bug where the non-gentle death of the recursive merge would prevent not only the message from being shown but *any* code to run after the failed merge, including rerere. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pull --rebase: reinstate helpful message on abort
Johannes Schindelin writes: > When calling `git pull --rebase`, things can go wrong. In such a case, > we want to tell the user about the most common ways out of this fix: > ... > builtin/am.c | 1 + > 1 file changed, 1 insertion(+) It is strange to see a patch to am that does not talk anything about it, though. And looking at the codepath, the issue does not have much to do with "pull --rebase". It doesn't even have much to do with "rebase". This is purely about "am -3" fallback codepath. Because fall-back-threeway wants to react to an error (i.e. calls merge_recursive_generic() and wants to use its return value), but merge_recursive_generic() can die, it fails to do so. It would not even run rerere(), for example. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
