On Mon, Mar 14, 2016 at 1:54 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Sat, Mar 12, 2016 at 1:41 AM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: >> Add commit.verbose configuration variable as a convenience for those >> who always prefer --verbose. >> >> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx> >> Mentored-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> > More typical would be to say Helped-by: rather than Mentored-by:. > > Also, place your sign-off last. Sure! > This is a bogus regular expression; some greps may barf on it > outright, while others will treat "*" as a literal. Either way, it > could never match "diff --git" so the test would succeed by accident > even if git-status did erroneously respect "commit.verbose". I was unaware about this. Thanks for pointing it out > I realize that it's possible to (mis)read Junio's recommendation[1] > about also testing git-status as a hint to combine the git-commit and > git-status checks into a single test, but that's not what was > intended. These are conceptually distinct checks, thus they should > each have a separate test. I'd suggest restoring the > "commit.verbose=true and --verbose omitted" test as it was shown in > [2], and then adding a new test in this script for git-status which > looks something like this: > > test_expect_success 'status ignores commit.verbose=true' ' > git status >actual && > ! grep "^diff --git" actual > ' I misread Junio's recommendation. Will change this. > Not at all mandatory, but it wouldn't hurt to add a couple additional tests: > > * commit.verbose=true and --verbose > * commit.verbose=false and --no-verbose I was thinking of putting these tests as when I was debugging (simply by printing verbose variable), I found that when commit.verbose=true and --verbose the value of the variable `verbose` is 2. But then I thought it wouldn't be that useful. But since you have pointed it out now, I will definitely include them. Regards, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html