Re: [PATCH v3] Isolate If-Modified-Since handling in gitweb

From: Jakub Narebski <jnareb@gmail.com>
Date: Mar 22, 2012, 13:05 UTC
Message-ID: <201203221405.13193.jnareb@gmail.com>
In-Reply-To: <20120321173824.GA31490@odin.tremily.us>

On Wed, 21 Mar 2012, W. Trevor King wrote:

> On Wed, Mar 21, 2012 at 05:55:06PM +0100, Jakub Narebski wrote:

Show 5 quoted lines

> > There is a tool to create patches to send: git-format-patch.  Myself I
> > usually create a new directory for a patch series, e.g. mdir.if_mod.v3,
> > and use git-format-patch to populate it, e.g.
> 
> Ah, I like that.  Then I can rebase away ;).

Yep.  Or use some patch / mailqueue management interface like StGit.

Show 8 quoted lines

> > I think it would be better to add initial tests with refactoring, and
> > snapshot specific tests with snapshot support, e.g.:
> > 
> >   1/2: gitweb: Refactor If-Modified-Since handling and add tests
> >   2/2: gitweb: Add If-Modified-Since support for snapshots
> 
> But the new tests would be for the new functionality (i.e. snapshot
> support), so they wouldn't belong in the general refactoring commit.

I was thinking that "and add tests" would be about 'feed' action test and i-m-s.

Show 13 quoted lines

> > Currently all of those work
> > 
> >     http://.../gitweb.cgi?p=git.git;a=snapshot;h=v1.7.6;sf=tgz
> >     http://.../gitweb.cgi?p=git.git;a=snapshot;h=f696543d;sf=tgz"
> >     http://.../gitweb.cgi?p=git.git;a=snapshot;h=b1485af8;sf=tgz"
> > 
> > v1.7.6 is a tag, f696543d is a commit (v1.7.6^{}), b1485af8 is a tree
> > (v1.7.6^{tree}).
> > 
> > The last URL would stop working after your change with 404
> > "Unknown commit object".
> 
> Indeed it does.  I'll add you're suggested skipping for these cases.

If by "skipping" you mean here turning off handling of If-Modified-Since
and Last-Modified headers, then I agree.

> There should be a way to determine the oldest commit refering to a
> tree, which could be used for timestamping tree-ishes, but that can be
> a project for another day ;).

That also can be ambiguous.

-- 
Jakub Narebski
Poland