From: Leslie Cheng Date: Thu, 22 Feb 2024 03:04:38 GMT Subject: Re: [PATCH] Add unix domain socket support to HTTP transport. Message-ID: <5a857e76-659d-4d94-ae54-f5c22c9917c6@gmail.com> In-Reply-To: <20240221220945.M686016@dcvr> > No need for trailing `.' in commit message titles Will fix in the next patch, sorry! > Personally, I'd hoist the #ifdef part into a standalone function > since I find mixing CPP and C conditionals confusing. > > disclaimer: I'm an easily confused person and don't usually > program in C, though. I considered extracting it out, but the other conditionals in this function follow a similar pattern so I didn't want to change it. However, my use here is also the first time there's both an #ifdef and nested #ifndef, which I agree makes it a bit confusing to grok. I'm open to changing it, but I'll let it sit and marinate for a bit. > `nc' isn't widely installed, its supported flags vary between > implementations, and our test suite doesn't currently use it. > I suggest either using a small bit of Perl or writing a t/helper > program to do its job. > > Finally, hard tabs should be used for indentation throughout. > > I'll wait on others to comment since I haven't looked at git > hacking in a while. > > Anyways, I think this feature could be useful for me, too :> > Thanks. Good catch, I'll fix in the next patch. I've subbed `nc` out for a simple Perl script to pipe back and forth, just making sure CI is happy about this before submitting.