Re: Improving merge messages for 1.7.10 and making "pull" easier

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 25, 2012, 09:27 UTC
Message-ID: <CACBZZX7rejo-fXdLp9SNU=KpVs+1gk=ux=aEqJcN38B+pXi2tQ@mail.gmail.com>
In-Reply-To: <7vy5rsyq9w.fsf@alter.siamese.dyndns.org>

On Fri, Feb 24, 2012 at 20:39, Junio C Hamano <gitster@pobox.com> wrote:

Show 12 quoted lines

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Firstly (and as a more general thing) I think we should add a mention
>> of "git merge --abort" to the message, just saving an empty file is
>> not sufficient to fully clear the merge state:
>
> Makes sense, but the new message does not quite parse.
>
>>> "Lines starting with '#' will be ignored, and an empty message followed\n"
>>> "by 'git merge --abort' the merge.\n");
>
> Perhaps s/the merge/aborts &/ or something.

Yeah, it needs better wording.

Show 14 quoted lines

>> Additionally, perhaps it would be a good idea to:
>>
>>  * Detect if the user didn't run this explicitly but implicitly from a
>>    "git pull". We could pass some env var along or another option
>>    (e.g. --internal-from-porcelain=pull) and add this:
>>
>>        You've merged implicitly via a "git pull", if you're just
>>        updating some local work in progress to keep up with upstream
>>        you may want to use "git pull --rebase" instead (or set the
>>        pull.rebase configuration variable) to rebase instead of merge.
>
> Won't this message be given to _all_ users of "git pull", even to the ones
> who already have decided correctly that "pull" is the right thing in their
> situation?  With a new advice.* settings to squelch it, perhaps.

Yeah, I'm not sure it's a good idea to add it given that.

Show 35 quoted lines

>>  * Explicitly check if we're merging an updated upstream into the
>>    work-in-progress topic,...
>
> It might be a worthy goal, but how would we detect it?  A few examples
> that we shouldn't give an unhelpful advice with a false positive are
> merges into:
>
>  - The 'master' branch used by people who use Git as an improved CVS, when
>   they do an equivalent of 'cvs update'.  Merging the updated 'master'
>   from the central repository into their 'master' that contains their
>   work that may or may not be ready to be pushed back is how their
>   project works.  It is a norm for them to make such a merge, even though
>   more experienced people may prefer to see the history of their project
>   kept cleaner by suggesting their project participants to use their own
>   topic branches.
>
>  - Integration branches like my 'next', when it gets a merge from
>   'master'. This is "merging an updated upstream" but is done in order to
>   keep the promise that 'next' would contain everything in 'master'.
>
> And what alternative would we offer?  If we were to suggest "rebase", we
> would also need to consider the topic of the other a-couple-of-days-old
> thread to detect which part of history is no longer subject to rewrite.
>
>> I work with a lot of inexperienced git users and a lot of them are
>> going to be very confused by this change. I still think it's a good
>> change to make, but we could do a lot more to mitigate the inevitable
>> confusion.
>
> What exact change are you talking about with "this change"?  Earlier you
> had a chance to edit the merge log only when it needed your help resolving
> (hence you did a separate "git commit" to record it) but you had to "git
> commit --amend" (or start with "git merge --no-commit") to edit the merge
> log if it did not need any help resolving conflicts, but now you do not
> have to.  Is that the change you have in mind?

Yes, or more explicitly if you do:

  git commit
  <your upstream moves forward>
  git pull

You'll now be presented with an editor asking you to enter a merge message, whereas before it just silently created a merge commit.

> I would like to know how that would lead to an "inevitable confusion".
> Admittedly, the original without any "# Please do X" comment, the user may
> wonder what is being asked of him when he sees the editor for the first
> time, but I thought Thomas's patch took care of that issue.

I mean that people who use this babytalk subset of git and have been doing so for months/years without having editor dialogs pop up every time they pull are going to be confused when that suddenly starts happening.

Show 10 quoted lines

>> One thing that would help these users in particular would be to have
>> some easy to use replacement for their frequent use of "git
>> pull".
>
> After this part, I think you shifted into a different topic.
>
> I have mixed feelings about "rebase your unpublished work and keep it
> always a descendant of the upstream" workflow you seem to be advocating.
> It _might_ deserve a bit more visibility, but I do not think rewording
> this message done during "merge" is the place to do so.

Yeah, agreed.

Show 5 quoted lines

>> They don't often commit their work (because of git inexperience) so
>> rebasing will error out because the tree is unclean.
>
> That is a *good* thing, isn't it?  There lies the perfect opportunity for
> them to train their fingers to commit first and then rebase.

That's true, anyway I'll submit an improved patch for adding --abort to the current message. I think the other suggestions either aren't doable or weren't appropriate.