At 2024-07-31 15:24-0700, Junio C Hamano [off-list ref] sent:

In a block with local variable decl, be more friendly to readers by
having a blank line between the end of declarations and the first
statement.

Will do.

We insist that it must be "localhost", so let's not do strcasecmp()
but just do strcmp().

I don't see the wisdom of being more restrictive than curl is in this 
respect. The documentation makes the claim that curl's syntax is what this 
option supports, and while that is not literally true due to how protocols 
are handled, I don't see a reason to intentionally deviate further.

I've tested that curl does the right thing with any casing of localhost, 
both on its own and via Git. Host names are generally case insensitive; 
the error message should be understood in that context. And if it is 
misunderstood, there's no negative impact on the user who writes 
"localhost" (while there is a negative impact on the user who quite 
reasonably expects "LOCALHOST" to work if we don't follow curl's lead).

Making it a regular test_expect_success would mean GIT_SKIP_TEST
mechansim can be used to skip it, which is probably not what you
want.  Can't this be a more common test_lazy_prereq, perhaps like

	test_lazy_prereq SOCKS_PROXY '
		# useful comment about 30% here ...
		test_have_prereq PERL &&
		start_socks %30.sock
	'

or something?

This existing test file is definitely not written with running individual 
tests in mind; the first test is a prerequisite for all that comes after, 
and the second test seems to be required for tests 3–5 as well.

But sure, I'll use that pattern if you like.

R