{"thread":{"id":"29720","subject":"Improving merge messages for 1.7.10 and making \"pull\" easier","startedAt":"2012-02-24T09:59:00Z","lastAt":"2012-02-25T09:27:55Z","messageCount":3,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"185324","messageId":"CACBZZX5UVq9k7jvSy3m+yMVj7_JbfLp8ugFWf2gGFdMz_8GPEA@mail.gmail.com","threadId":"29720","inReplyTo":null,"subject":"Improving merge messages for 1.7.10 and making \"pull\" easier","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-02-24T09:59:00Z","receivedAt":"2012-02-24T09:59:00Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"With 1.7.10 we'll get merge edit messages by default (which I like)\nbut I wonder if we could better clarify this to users when they see a\nmerge message pop up.\n\nFirstly (and as a more general thing) I think we should add a mention\nof \"git merge --abort\" to the message, just saving an empty file is\nnot sufficient to fully clear the merge state:\n\n    diff --git a/builtin/merge.c b/builtin/merge.c\n    index d3e1e8d..9169c74 100644\n    --- a/builtin/merge.c\n    +++ b/builtin/merge.c\n    @@ -889,8 +889,8 @@ static const char merge_editor_comment[] =\n     N_(\"Please enter a commit message to explain why this merge is\nnecessary,\\n\"\n        \"especially if it merges an updated upstream into a topic branch.\\n\"\n        \"\\n\"\n    -   \"Lines starting with '#' will be ignored, and an empty message aborts\\n\"\n    -   \"the commit.\\n\");\n    +   \"Lines starting with '#' will be ignored, and an empty message\nfollowed\\n\"\n    +   \"by 'git merge --abort' the merge.\\n\");\n\n     static void prepare_to_commit(void)\n     {\n\nAdditionally, perhaps it would be a good idea to:\n\n * Detect if the user didn't run this explicitly but implicitly from a\n   \"git pull\". We could pass some env var along or another option\n   (e.g. --internal-from-porcelain=pull) and add this:\n\n       You've merged implicitly via a \"git pull\", if you're just\n       updating some local work in progress to keep up with upstream\n       you may want to use \"git pull --rebase\" instead (or set the\n       pull.rebase configuration variable) to rebase instead of merge.\n\n * Explicitly check if we're merging an updated upstream into the\n   work-in-progress topic, then mentioning some succinct version of\n   Junio's #1 point at\n   http://git-blame.blogspot.com/2012/02/anticipating-git-1710.html\n\nI work with a lot of inexperienced git users and a lot of them are\ngoing to be very confused by this change. I still think it's a good\nchange to make, but we could do a lot more to mitigate the inevitable\nconfusion.\n\nOne thing that would help these users in particular would be to have\nsome easy to use replacement for their frequent use of \"git\npull\".\n\nThey don't often commit their work (because of git inexperience) so\nrebasing will error out because the tree is unclean. I tried to hack\nsupport for unclean trees into git-rebase(1) the other day but it was\ndifficult because at various points in the process it'll do a \"git\nreset --hard\".\n\nMaybe having an option to \"git pull\" that would make it do:\n\n    git stash save --include-untracked -- \"Automatically created stash\nby \\\"git pull\\\" at $(date)\"\n    git pull --rebase\n    git stash pop\n\nWould make things easier. It could be called \"git pull --rebase\n--stash\".\n"},{"id":"185347","messageId":"7vy5rsyq9w.fsf@alter.siamese.dyndns.org","threadId":"29720","inReplyTo":"CACBZZX5UVq9k7jvSy3m+yMVj7_JbfLp8ugFWf2gGFdMz_8GPEA@mail.gmail.com","subject":"Re: Improving merge messages for 1.7.10 and making \"pull\" easier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-24T19:39:39Z","receivedAt":"2012-02-24T19:39:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> Firstly (and as a more general thing) I think we should add a mention\n> of \"git merge --abort\" to the message, just saving an empty file is\n> not sufficient to fully clear the merge state:\n\nMakes sense, but the new message does not quite parse.\n\n>> \"Lines starting with '#' will be ignored, and an empty message followed\\n\"\n>> \"by 'git merge --abort' the merge.\\n\");\n\nPerhaps s/the merge/aborts &/ or something.\n\n> Additionally, perhaps it would be a good idea to:\n>\n>  * Detect if the user didn't run this explicitly but implicitly from a\n>    \"git pull\". We could pass some env var along or another option\n>    (e.g. --internal-from-porcelain=pull) and add this:\n>\n>        You've merged implicitly via a \"git pull\", if you're just\n>        updating some local work in progress to keep up with upstream\n>        you may want to use \"git pull --rebase\" instead (or set the\n>        pull.rebase configuration variable) to rebase instead of merge.\n\nWon't this message be given to _all_ users of \"git pull\", even to the ones\nwho already have decided correctly that \"pull\" is the right thing in their\nsituation?  With a new advice.* settings to squelch it, perhaps.\n\n>  * Explicitly check if we're merging an updated upstream into the\n>    work-in-progress topic,...\n\nIt might be a worthy goal, but how would we detect it?  A few examples\nthat we shouldn't give an unhelpful advice with a false positive are\nmerges into:\n\n - The 'master' branch used by people who use Git as an improved CVS, when\n   they do an equivalent of 'cvs update'.  Merging the updated 'master'\n   from the central repository into their 'master' that contains their\n   work that may or may not be ready to be pushed back is how their\n   project works.  It is a norm for them to make such a merge, even though\n   more experienced people may prefer to see the history of their project\n   kept cleaner by suggesting their project participants to use their own\n   topic branches.\n\n - Integration branches like my 'next', when it gets a merge from\n   'master'. This is \"merging an updated upstream\" but is done in order to\n   keep the promise that 'next' would contain everything in 'master'.\n\nAnd what alternative would we offer?  If we were to suggest \"rebase\", we\nwould also need to consider the topic of the other a-couple-of-days-old\nthread to detect which part of history is no longer subject to rewrite.\n\n> I work with a lot of inexperienced git users and a lot of them are\n> going to be very confused by this change. I still think it's a good\n> change to make, but we could do a lot more to mitigate the inevitable\n> confusion.\n\nWhat exact change are you talking about with \"this change\"?  Earlier you\nhad a chance to edit the merge log only when it needed your help resolving\n(hence you did a separate \"git commit\" to record it) but you had to \"git\ncommit --amend\" (or start with \"git merge --no-commit\") to edit the merge\nlog if it did not need any help resolving conflicts, but now you do not\nhave to.  Is that the change you have in mind?\n\nI would like to know how that would lead to an \"inevitable confusion\".\nAdmittedly, the original without any \"# Please do X\" comment, the user may\nwonder what is being asked of him when he sees the editor for the first\ntime, but I thought Thomas's patch took care of that issue.\n\n> One thing that would help these users in particular would be to have\n> some easy to use replacement for their frequent use of \"git\n> pull\".\n\nAfter this part, I think you shifted into a different topic.\n\nI have mixed feelings about \"rebase your unpublished work and keep it\nalways a descendant of the upstream\" workflow you seem to be advocating.\nIt _might_ deserve a bit more visibility, but I do not think rewording\nthis message done during \"merge\" is the place to do so.\n\n> They don't often commit their work (because of git inexperience) so\n> rebasing will error out because the tree is unclean.\n\nThat is a *good* thing, isn't it?  There lies the perfect opportunity for\nthem to train their fingers to commit first and then rebase.\n"},{"id":"185410","messageId":"CACBZZX7rejo-fXdLp9SNU=KpVs+1gk=ux=aEqJcN38B+pXi2tQ@mail.gmail.com","threadId":"29720","inReplyTo":"7vy5rsyq9w.fsf@alter.siamese.dyndns.org","subject":"Re: Improving merge messages for 1.7.10 and making \"pull\" easier","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2012-02-25T09:27:55Z","receivedAt":"2012-02-25T09:27:55Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Fri, Feb 24, 2012 at 20:39, Junio C Hamano <gitster@pobox.com> wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n>\n>> Firstly (and as a more general thing) I think we should add a mention\n>> of \"git merge --abort\" to the message, just saving an empty file is\n>> not sufficient to fully clear the merge state:\n>\n> Makes sense, but the new message does not quite parse.\n>\n>>> \"Lines starting with '#' will be ignored, and an empty message followed\\n\"\n>>> \"by 'git merge --abort' the merge.\\n\");\n>\n> Perhaps s/the merge/aborts &/ or something.\n\nYeah, it needs better wording.\n\n>> Additionally, perhaps it would be a good idea to:\n>>\n>>  * Detect if the user didn't run this explicitly but implicitly from a\n>>    \"git pull\". We could pass some env var along or another option\n>>    (e.g. --internal-from-porcelain=pull) and add this:\n>>\n>>        You've merged implicitly via a \"git pull\", if you're just\n>>        updating some local work in progress to keep up with upstream\n>>        you may want to use \"git pull --rebase\" instead (or set the\n>>        pull.rebase configuration variable) to rebase instead of merge.\n>\n> Won't this message be given to _all_ users of \"git pull\", even to the ones\n> who already have decided correctly that \"pull\" is the right thing in their\n> situation?  With a new advice.* settings to squelch it, perhaps.\n\nYeah, I'm not sure it's a good idea to add it given that.\n\n>>  * Explicitly check if we're merging an updated upstream into the\n>>    work-in-progress topic,...\n>\n> It might be a worthy goal, but how would we detect it?  A few examples\n> that we shouldn't give an unhelpful advice with a false positive are\n> merges into:\n>\n>  - The 'master' branch used by people who use Git as an improved CVS, when\n>   they do an equivalent of 'cvs update'.  Merging the updated 'master'\n>   from the central repository into their 'master' that contains their\n>   work that may or may not be ready to be pushed back is how their\n>   project works.  It is a norm for them to make such a merge, even though\n>   more experienced people may prefer to see the history of their project\n>   kept cleaner by suggesting their project participants to use their own\n>   topic branches.\n>\n>  - Integration branches like my 'next', when it gets a merge from\n>   'master'. This is \"merging an updated upstream\" but is done in order to\n>   keep the promise that 'next' would contain everything in 'master'.\n>\n> And what alternative would we offer?  If we were to suggest \"rebase\", we\n> would also need to consider the topic of the other a-couple-of-days-old\n> thread to detect which part of history is no longer subject to rewrite.\n>\n>> I work with a lot of inexperienced git users and a lot of them are\n>> going to be very confused by this change. I still think it's a good\n>> change to make, but we could do a lot more to mitigate the inevitable\n>> confusion.\n>\n> What exact change are you talking about with \"this change\"?  Earlier you\n> had a chance to edit the merge log only when it needed your help resolving\n> (hence you did a separate \"git commit\" to record it) but you had to \"git\n> commit --amend\" (or start with \"git merge --no-commit\") to edit the merge\n> log if it did not need any help resolving conflicts, but now you do not\n> have to.  Is that the change you have in mind?\n\nYes, or more explicitly if you do:\n\n  git commit\n  <your upstream moves forward>\n  git pull\n\nYou'll now be presented with an editor asking you to enter a merge\nmessage, whereas before it just silently created a merge commit.\n\n> I would like to know how that would lead to an \"inevitable confusion\".\n> Admittedly, the original without any \"# Please do X\" comment, the user may\n> wonder what is being asked of him when he sees the editor for the first\n> time, but I thought Thomas's patch took care of that issue.\n\nI mean that people who use this babytalk subset of git and have been\ndoing so for months/years without having editor dialogs pop up every\ntime they pull are going to be confused when that suddenly starts\nhappening.\n\n>> One thing that would help these users in particular would be to have\n>> some easy to use replacement for their frequent use of \"git\n>> pull\".\n>\n> After this part, I think you shifted into a different topic.\n>\n> I have mixed feelings about \"rebase your unpublished work and keep it\n> always a descendant of the upstream\" workflow you seem to be advocating.\n> It _might_ deserve a bit more visibility, but I do not think rewording\n> this message done during \"merge\" is the place to do so.\n\nYeah, agreed.\n\n>> They don't often commit their work (because of git inexperience) so\n>> rebasing will error out because the tree is unclean.\n>\n> That is a *good* thing, isn't it?  There lies the perfect opportunity for\n> them to train their fingers to commit first and then rebase.\n\nThat's true, anyway I'll submit an improved patch for adding --abort\nto the current message. I think the other suggestions either aren't\ndoable or weren't appropriate.\n"}]}