Re: [PATCHv3 4/4] am, rebase: teach quiet option
- From
Stephen Boyd <bebarino@gmail.com>
- Date
- Jun 16, 2009, 07:50 UTC
- Message-ID
- <4A374EAD.9030309@gmail.com>
- In-Reply-To
- <7vbpoog1py.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
Show 12 quoted lines
> 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.
Show 5 quoted lines
> 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...
Show 21 quoted lines
> 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.