Re: [PATCH v4] commit: add a commit.verbose config variable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: "Pranit Bauva" <pranit.bauva@xxxxxxxxx>
On Fri, Mar 11, 2016 at 4:31 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:

    Add commit.verbose configuration variable as a convenience
    for those who always prefer --verbose.

or something.

Sure!

As a convenience to reviewers, please use this area below the "---"
line to provide links and explain what changed since the previous
round rather than doing so in a separate email.

Actually I am sending the patches with submitGit herokuapp because my
institute proxy does not allow IMAP/POP3 connections.

You can still include the 'three dashes' and a commentary at the end of your (local) regular commit message, and then when it is sent as a patch it will have the right format. There are carried through rebases as well.

There is a similar feature for attaching notes (though I haven't used it).

Either should get around your institute's proxy issue.


The "permanently" bit sounds scary. A more concise way to state this might be:

    See the `commit.verbose` configuration variable in
    linkgit:git-config[1].

which doesn't bother spelling out what the intelligent reader should
infer from the reference.
Style: space before {

Sure!

+test_expect_success 'commit with commit.verbose true and no arguments' '

"no arguments" doesn't convey much; how about "--verbose omitted" or
something? Ditto for the titles of other tests.

Will change the language construct.
+       echo content >file &&
+       git add file &&
+       test_config commit.verbose true &&
+       (
+               GIT_EDITOR=cat &&
+               export GIT_EDITOR &&
+               test_must_fail git commit >output
+       ) &&
+       test_i18ngrep "diff --git" output
+'

Making git-commit fail unconditionally with "aborting due to empty
commit message" is a rather sneaky way to perform this test. I would
have expected to see these new tests re-use the existing machinery
provided by this script (the check-for-diff "editor") rather than
inventing an entirely new and unintuitive mechanism. Doing so would
also reduce the size of each new test.

I agree on the fact that making git-commit fail unconditionally is not
a good way to perform the test. "check-for-diff" is not really an
"editor" and it checks for the commit message after it has been
written to the history. The verbose output is stripped when it is
written to the history so we won't be able to test whether this patch
works. This is where purposely breaking the code is required as when
the commit fails, it gives the output of the contents present at that
time (which will contain the verbose output). More over the
'check-for-diff' uses grep which is not preferred. Many tests are now
using test_i18ngrep (eg. f79ce8db). I had planned on using
'check-for-diff' before but it took me some time to figure out this
behavior and thus I began searching for another mechanism (breaking
code).

Some additional tests[1][2] are probably warranted.

[1]: http://article.gmane.org/gmane.comp.version-control.git/288648
[2]: http://article.gmane.org/gmane.comp.version-control.git/288657

I think these tests also are better included in this file as this
patch triggers it and it would not make much of a difference between
t7507 and t7502 but in fact improve its readability.
 test_done

--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]