Re: [RFC PATCH] gitweb: use HEAD as primary sort key in git_get_heads_list()

From: Jeff King <peff@peff.net>
Date: Jun 8, 2021, 22:07 UTC
Message-ID: <YL/qFDPoF/0+ArAV@coredump.intra.peff.net>
In-Reply-To: <20210608211440.37985-1-greg@hurrell.net>

On Tue, Jun 08, 2021 at 11:14:40PM +0200, Greg Hurrell wrote:

Show 13 quoted lines

> Prior to this commit, the "heads" section on a gitweb summary page would
> list the heads in `-committerdate` order (ie. the most recently-modified
> ones at the top), tie-breaking equal-dated refs using the implicit
> `refname` sort fallback.
> 
> This commit adds another `--sort` parameter to the `git for-each-ref`
> invocation in `git_get_heads_list()`, ensuring that the `HEAD` ref
> always ends up getting sorted to the top, seeing as it is typically the
> "primary" line of development in some sense.
> 
> This seems to be a useful change, because I can't see anywhere else in
> the gitweb UI where we actually indicate to the user what the "default"
> branch is (ie. what they'll checkout if they run `git clone`).

Your use of "seems" in the final paragraph is a leftover from the earlier commit message, and I think weakens your message. It's OK to assert that it really _is_ a useful change, I would say. :)

This patch looks good to me overall. In addition to dropping the RFC tag, you're more likely to get attention by having a subject line without "Re:" in it (so people realize it's a patch to look at, and not just a continuation of the discussion).

Show 19 quoted lines

> On Tue, Jun 8, 2021, at 11:02 AM, Greg Hurrell wrote:
> > On Tue, Jun 8, 2021, at 10:34 AM, Jeff King wrote:
> > >   1. break ties by name, like:
> > > 
> > >        git for-each-ref --sort=refname --sort=-committerdate
> > > 
> > >   2. emphasize the HEAD branch, even if it isn't the newest:
> > > 
> > >        git for-each-ref --sort=refname --sort=-committerdate --sort=-HEAD
> 
> I was wracking my brains over this one trying to figure out why
> it wasn't already doing the right thing based on what I see in
> ref-filter.c.  It sure looks like the `--sort=refname` fallback should
> be automatic, but I wasn't seeing it happen in my gitweb instance.
> 
> Turns out there was a bug that you fixed in 7c5045fc180ed09ed4cb5 which
> made it in soon after v2.20.4 fixing a problem. I was seeing different
> behavior on gitweb running on Amazon Linux AMI, because that's still
> using Git v2.18.5.

Heh, OK. I almost suggested "gee, wouldn't it be nice if we used the refname as a fallback tie-breaker by default". You'd think I would either remember such fixes, or at least bother to look at the code. :)

> So, that means "1" isn't necessary. "2" is the only possibly interesting
> bit. I've reworded the commit text accordingly, still labeled as "RFC"
> to see if there is any consensus on this being a good idea or not.

Yep, I agree on all counts.

In my experience gitweb doesn't tend to get a lot of interest from reviewers, and I consider it mostly in maintenance mode these days. So be prepared for silence. In that case, I'd give it a few days and repost the patch to see if Junio is interested in picking it up.

-Peff