Re: [PATCH v8 3/3] gitweb: add If-Modified-Since handling to git_snapshot().

On Thu, 29 Mar 2012, W. Trevor King wrote:

Because snapshots can be large, you can save some bandwidth by
supporting caching via If-Modified-Since.  This patch adds support for
the i-m-s request to git_snapshot() if the request is a commit.

Perhaps it is worth clarifying that "caching" here means external HTTP
caching, either by web browser, or by web accelerator / reverse proxy.
Supporting Last-Modified and If-Modified-Since helps that[1][2].

[1]: http://www.mnot.net/cache_docs/#VALIDATE
[2]: http://www.mnot.net/cache_docs/#SCRIPT

Requests for snapshots of trees, which lack well defined timestamps,
are still handled as they were before.

"still handled as they were before" means "do not support l-m / i-m-s",
isn't it?  Wouldn't it be better to write it explicitely?

Signed-off-by: W Trevor King <redacted>

Anyway, I like those changes:

  Acked-by: Jakub Narebski [off-list ref]

[...]

+test_expect_success 'modification: tree snapshot' '
+	ID=`git rev-parse --verify HEAD^{tree}` &&
+	export HTTP_IF_MODIFIED_SINCE="Wed, 6 Apr 2005 22:14:13 +0000" &&
+	test_when_finished "unset HTTP_IF_MODIFIED_SINCE" &&
+	gitweb_run "p=.git;a=snapshot;h=$ID;sf=tgz" &&
+	grep "Status: 200 OK" gitweb.headers &&
+	! grep -i "last-modified" gitweb.headers

If we ignore case, we can write

  +	! grep -i "Last-Modified:" gitweb.headers

which is IMVVVHO slightly more readable.

Not that it matters much.  Just nitpicking.

+'
+test_debug 'cat gitweb.headers'

-- 
Jakub Narebski
Poland