Thread (1 message) 1 message, 1 author, 2017-08-22

Re: [PATCH v2 3/3] merge: save merge state earlier

From: Junio C Hamano <hidden>
Date: 2017-08-22 16:03:35

Michael J Gruber [off-list ref] writes:
quoted
Can squash ever be true in this function?

This function has two callsites: merge_trivial() and
finish_automerge().

I think merge_trivial() will not be called under "--squash", which
turns option_commit off and the only callsite of it is inside an
else-if clause that requres option_commit to be true.  You can do a
similar deduction around the "automerge_was_ok" variable to see if
finish_automerge() can be called when "--squash" is given; I suspect
the answer may be no.
I'll go without the if, after more testing.
I was sort of expecting that tracing the control flow would give us
the definite answer and that would be much better than any amount of
testing.

In any case, I wasn't even suggesting to remove "if".  It might even
be worth doing

	if (squash)
		BUG("the control must not reach here under --squash");
	write_emrge_heads(...);

if we know the control does not have to reach with "--squash" in
today's code, so that future careless refactoring does not break
this fix.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help