Thread (8 messages) flat view 8 messages, 3 authors, 2016-06-15

Re: [PATCH] Add the --submodule-summary option to the diff option family

From: Jens Lehmann <hidden>
Date: 2016-06-15 22:47:29

Johannes Schindelin schrieb:
quoted
quoted
I have no idea why "submodule --summary" uses --first-parent, but 
personally, I would _hate_ it not to see the merged commits in the 
diff.

For a summary, you might get away with seeing

	> Merge bla
	> Merge blub
	> Merge this
	> Merge that

but in a diff that does not cut it at all.
As long as bla/blub/this/that are descriptive enough, I do not see at all
why you think "summary" is Ok and "diff" is not.  If your response were
"it is just a matter of taste; to some people (or project) --first-parent
is useful and for others it is not", I would understand it, and it would
make sense to use (or not use) --first-parent consistently between this
codepath and "submodule --summary", though.
You may be used to git.git's quality of naming the branches you merge.

Sadly, this is not the common case.
IMHO both arguments are valid, using --first-parent really is a matter of
taste *and* it is dependent on the quality of branch naming whether it is
useful or not.

But when both commands shall produce the same output, i think we have to
use --first-parent as default, no? And maybe we could add another option
to diff which can change that behaviour according to users taste?

But I really, really, really want to avoid a fork() in the common case.  I 
do have some users on Windows, and I do have a few submodules in that 
project.  Having too many fork() calls there would just give Git a bad 
reputation.  And it has enough of that, it does not need more.
Me too thinks performance matters here. We do have a repo at my dayjob
with more than a handful of submodules and its main target platform is
windows ... so having that perform nicely is a win for us.


Jens
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help