git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: Junio C Hamano <junkio@cox.net>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>, git@vger.kernel.org
Subject: Re: [PATCH] merge-recursive: do not report the resulting tree object name
Date: Sat, 13 Jan 2007 00:14:47 -0500	[thread overview]
Message-ID: <20070113051447.GA22063@spearce.org> (raw)
In-Reply-To: <7vbql3pxz8.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
>         $ git merge jc/merge-base
>      1	Trying really trivial in-index merge...
>      2	fatal: Merge requires file-level merging
>      3	Nope.
>      4	Merging HEAD with jc/merge-base
>      5	Merging:
>      6	b60daf0 Make git-prune-packed a bit more chatty.
>      7	5b75a55 Teach "git-merge-base --check-ancestry" about refs.
>      8	found 1 common ancestor(s):
>      9	1c23d79 Don't die in git-http-fetch when fetching packs.
>     10	Auto-merging Makefile
>     11	Auto-merging builtin-branch.c
>     12	Auto-merging builtin-reflog.c
>     13	CONFLICT (content): Merge conflict in builtin-reflog.c
>     14	Auto-merging builtin.h
>     15	Auto-merging git.c
>     16	Removing merge-base.c
>     17	Resolved 'builtin-reflog.c' using previous resolution.
>     18	Automatic merge failed; fix conflicts and then commit the result.
> 
> Among these, I think lines 2..3 are somewhat confusing but I am
> used to seeing them and do not mind them too much.

In my experience these lines scare new users.  And then they start
to ignore other "fatal:" messages from Git because they can safely
ignore this particular one.  Not good.  One reason I like my patch
that's in next.

> Lines 4..9 do not have any real information that helps the end
> user (even though it would be a very good debugging aid for
> merge-recursive developers).

I agree.  I've grown used to seeing them and read it for
entertainment.  Clearly I need to get out more.  They probably
should be relegated to a GIT_MERGE_OPTIONS environment variable
flag or to a command line parameter, as they are probably only
useful when debugging the application itself.
 
> Lines 10..16 are useful, but I think we probably should show
> them only for outermost merges.

Actually I think that only 13 is useful.  10-12,14-17 are
pretty useless messages in my mind.  I really don't care that
merge-recursive automatically merged these files, as in all cases but
the one reported by line 13 the merge was successful.  The diffstat
that is normally displayed by git-merge after a successful merge
shows you what files were modified by the other branch.  It also
often causes the output of merge-recursive to scroll off the screen,
making those messages even less useful.

> An multi-base example:
>     16	Auto-merging gitweb/gitweb.perl
>     17	Merge made by recursive.
>     18	 gitweb/gitweb.css  |    2 +
>     19	 gitweb/gitweb.perl |  165 ++++++++++++++++++++++++++++++++...
>     20	 2 files changed, 117 insertions(+), 50 deletions(-)
> 
> I do not think we need to show 1..15 at all, perhaps without
> "export GIT_MERGE_BASE_DEBUG=YesPlease".

Yes, I agree.  Except I'd say 1..16, for the reason stated above.

But then I would like a progress meter, showing % of files resolved,
to keep the user entertained.  Alex has 1 min+ merges.  1 minute
of absolutely no feedback is not very nice to a new user.

Maybe when I'm done hacking on git-describe performance improvements
I'll look at merge-recursive.

-- 
Shawn.

  parent reply	other threads:[~2007-01-13  5:14 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-04 10:47 [PATCH] Speedup recursive by flushing index only once for all entries Alex Riesen
2007-01-04 12:33 ` Johannes Schindelin
2007-01-04 12:47   ` Alex Riesen
2007-01-04 20:22     ` Junio C Hamano
2007-01-05 11:22       ` Alex Riesen
2007-01-07 16:31         ` Alex Riesen
2007-01-10 18:06           ` Junio C Hamano
2007-01-10 19:28           ` Junio C Hamano
2007-01-10 22:11             ` Junio C Hamano
2007-01-10 23:07             ` Alex Riesen
2007-01-10 23:23               ` Linus Torvalds
2007-01-11  8:14                 ` Johannes Schindelin
2007-01-11  9:03                   ` Alex Riesen
2007-01-11 12:11                     ` Alex Riesen
2007-01-11 20:37                       ` Junio C Hamano
2007-01-11  9:02                 ` Alex Riesen
2007-01-11 16:38                   ` Linus Torvalds
2007-01-11 17:43                     ` Alex Riesen
2007-01-11 18:02                       ` Linus Torvalds
2007-01-11 21:48                         ` Alex Riesen
2007-01-11 20:23                     ` Junio C Hamano
2007-01-11 22:10                       ` Alex Riesen
2007-01-11 22:28                         ` Linus Torvalds
2007-01-11 23:53                           ` Junio C Hamano
2007-01-12  0:18                           ` Alex Riesen
2007-01-11  0:34               ` Junio C Hamano
2007-01-11  8:15             ` Johannes Schindelin
2007-01-12 15:48             ` Sergey Vlasov
2007-01-12 17:38               ` Alex Riesen
2007-01-12 20:37                 ` Sergey Vlasov
2007-01-12 18:23               ` Junio C Hamano
2007-01-12 20:09                 ` [PATCH] merge-recursive: do not report the resulting tree object name Junio C Hamano
2007-01-12 23:36                   ` Johannes Schindelin
2007-01-13  0:32                     ` Junio C Hamano
2007-01-13  0:57                       ` Jakub Narebski
2007-01-13 11:01                         ` Johannes Schindelin
2007-01-13  5:14                       ` Shawn O. Pearce [this message]
2007-01-13  7:03                         ` Junio C Hamano
2007-01-12 20:30                 ` [PATCH] Speedup recursive by flushing index only once for all entries Alex Riesen
2007-01-12 21:07                 ` Sergey Vlasov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070113051447.GA22063@spearce.org \
    --to=spearce@spearce.org \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).