On Fri, Mar 25, 2016 at 5:35 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > On Thu, Mar 24, 2016 at 4:25 AM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote: >> Add commit.verbose configuration variable as a convenience for those >> who always prefer --verbose. > > The implementation looks better in this version. A couple comments > below about the test script... > >> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx> >> --- >> diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh >> @@ -9,6 +9,12 @@ test $(wc -l <out) = 1 >> EOF >> chmod +x check-for-diff > > Mentioned in patch 2/3 review, this patch (3/3) is where you should > update 'check-for-diff' to also check the line count, and the commit > message should explain the reason for doing so (and don't forget to > mention that it won't harm existing clients of 'check-for-diff'). Agree that check the line count should belong to this patch. And will add more details in the commit message. >> +write_script "check-for-double-diff" <<-\EOF && >> +grep '# Changes not staged for commit' "$1" >out && >> +test $(wc -l <out) = 2 >> +EOF >> +chmod +x check-for-double-diff > > Also mentioned in patch 2/3 review: drop 'chmod'; write_script() does > it for you. -- 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