Re: [PATCH] http-backend: remove a duplicated code branch

Jeff King [off-list ref] writes:

quoted

diff --git a/http-backend.c b/http-backend.c
index e7c0eeab23..3d6e2ff17f 100644
--- a/http-backend.c
+++ b/http-backend.c

@@ -466,9 +466,7 @@ static void run_service(const char **argv, int buffer_input)
 	struct child_process cld = CHILD_PROCESS_INIT;
 	ssize_t req_len = get_content_length();
 
-	if (encoding && !strcmp(encoding, "gzip"))
-		gzipped_request = 1;
-	else if (encoding && !strcmp(encoding, "x-gzip"))
+	if (encoding && (!strcmp(encoding, "gzip") || !strcmp(encoding, "x-gzip")))
 		gzipped_request = 1;

I think this conversion is correct, and I do find the resulting slightly
easier to read. I wondered if the two conditions might have come
separately, but no, they were both there in the initial 556cfa3b6d
(Smart fetch and push over HTTP: server side, 2009-10-30).

We do frown a bit on making small style changes like this. This kind of
churn isn't dramatically improving the quality of the code, and it
carries the risk of regression (if there is something subtle that you or
the reviewers missed) and creates a maintenance burden (it may conflict
with other patches, though I doubt it in this case, and it creates work
for reviewers and the maintainer to apply).

So...I dunno. I don't mind it, but it is not a pattern we like to
encourage in general. Let's see what Junio thinks.

This particular conversion is mostly "Meh", although if it were done
in this way:

	if (encoding) {
		if (!strcmp(encoding, "gzip") || !strcmp(encoding, "x-gzip"))
			gzipped_request = 1;
	}

the result may be a bit less "meh" by making it easier to adjust the
code for other kinds of encoding in the future.

My usual rule of thumb for such a single-hunk clean-up patch that is
"obviously not incorrect" is to take it only once or twice per new
contributor.  At some point, new contributors gain enough experience
to know better, at which point I'd start frowning at them.

Thanks.