Thread (17 messages) 17 messages, 3 authors, 2016-06-15

Re: [PATCHv3 4/4] am, rebase: teach quiet option

flat view

From: Stephen Boyd <hidden>
Date: 2016-06-15 22:46:57

Junio C Hamano wrote:
Hmm, this long conditional body looks ugly, and I suspect it is harder to
maintain than necessary.  Can we do something about it?

	# When we are allowed to fall back to 3-way later, don't give
        # false errors during the initial attempt.
	squelch=
	if test "$threeway" = t && test -n "$GIT_QUIET"
	then
		squelch='>/dev/null 2>&1 '
	fi
        eval "git apply $squelch$git_apply_opt"' --index "$dotest/patch"'
Thanks. I thought there would be a nicer way but I didn't know it.
Ah, I was blind.

While sending non-error messages to stderr is justifiable, I do not think
this one is, because all the other progress-y message in this program we
reviewed so far go to stdout.  I think we should drop >&2 here.
Will do, gotta fixup some tests for that though...
There is one more thing to think about in git-am, which I do not think you
addressed.  Consider this scenario.

    (1) Tell am to run quietly, feeding a four-patch series.

	$ git am -q -3 mbox

    (2) The first patch applies cleanly; the second one does not apply,
        even with -3, and leaves conflict (you did the right thing not to
        squelch the message when this happens).

    (3) You fix it up, and save the result from your editor, and tell it
        to continue.

	$ git am --resolved

Now, should the second invocation be also quiet, or talkative as default?

Note that the third and fourth patch are applied with -3 option in effect,
even though you did not say -3 when you restarted "am" with --resolved
(cf. ll.280-340 in git-am.sh).
Thanks for bringing this up. I gave this some thought before I sent the
series out but I was secretly hoping nobody would care :-) I think it's
more correct and maybe even more consistent to keep the quiet option
enabled.

Seeing as this series is going for round 4 I'll make sure to include
this as well.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help