Thread (1 message) 1 message, 1 author, 2017-08-10

Re: [PATCH v3 2/5] interpret-trailers: add an option to show only the trailers

From: Junio C Hamano <hidden>
Date: 2017-08-10 18:49:27

Jeff King [off-list ref] writes:
On Thu, Aug 10, 2017 at 11:28:52AM -0700, Stefan Beller wrote:
quoted
quoted
+test_expect_success 'only trailers' '
+       git config trailer.sign.command "echo config-value" &&
You may want to use 'test_config' here, which keeps the config
only for one test. The subsequent tests seem to overwrite the
config, so this is not wrong, just not the best style.
Yeah, I actually considered that but decided to keep style with the rest
of the script. I agree the whole thing could possibly switch to
test_config, but I suspect there may be some fallouts (the style of the
rest of the script seems to assume some continuity between the tests).
I agree with your judgment here.  As you said in the "use tool to
enforce consistent style" thread, sometimes humans need to apply
better taste than mechanical tooling could, and I view this one of a
good example.  

Even though this is not a C coding-style thing, but the essense of
the problem is the same.  That is one of the reasons why I earlier
said that we may see more style-only critique to patches if we
introduce a new tool without setting the expectation of what we want
to get out of such a tool straight.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help