Hi Junio On Wed, 19 Jun 2019 20:40:50 -0700 Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Two comments. > > The places touched by patch 1/5 emitted the error followed by an > advice message; this new one breaks the pattern by giving the "hint:" > first and then error. Be consistent by swapping these two (and > return -1, as "error() that returns -1" will no longer be the last > thing executed in this function. Yes, it also makes sense otherwise, but this will be more consistent. > This one, and the in_progress_advice emitted from the patch 1/5, are > both bad in that they make calls to advise() without guarding it > with an advice.* configuration variable. This does not allow the > user to say say "I've learned this part of Git enough; do not tell > me verbosely." Oh~ I was missing this point from the start. I'll make proper changes to my patch. > Pick a random global variable that is defined near the top of > advice.c, and learn how they are set (to true by default, allowing > configuration to flip it off) and how they are used in order to > prevent a call to advise() getting made. Then mimick that to guard > these calls to advise(). Ok, I'll look into it and change accordingly. > Thanks. Thanks Rohit