Re: [RFC PATCH] gitweb: use HEAD as primary sort key in git_get_heads_list()
To
Greg Hurrell
Cc
git@vger.kernel.org
From
Jeff King
See Also
Prev Ref 1
Date
2021-06-08 22:07:16 UTC
On Tue, Jun 08, 2021 at 11:14:40PM +0200, Greg Hurrell wrote:

> 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).

> 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