All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Greg Hurrell <greg@hurrell.net>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH] gitweb: use HEAD as primary sort key in git_get_heads_list()
Date: Tue, 8 Jun 2021 18:07:16 -0400	[thread overview]
Message-ID: <YL/qFDPoF/0+ArAV@coredump.intra.peff.net> (raw)
In-Reply-To: <20210608211440.37985-1-greg@hurrell.net>

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

  reply	other threads:[~2021-06-08 22:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-06  8:51 [RFC PATCH] gitweb: use HEAD as primary sort key in git_get_heads_list() Greg Hurrell
2021-06-06  8:57 ` Greg Hurrell
2021-06-08  8:34   ` Jeff King
2021-06-08  9:02     ` Greg Hurrell
2021-06-08 21:14       ` Greg Hurrell
2021-06-08 22:07         ` Jeff King [this message]
2021-06-09  0:15         ` Junio C Hamano
2021-06-09  7:38           ` Greg Hurrell
2021-06-09  7:47             ` Junio C Hamano
2021-06-09 19:28               ` [PATCH v2] gitweb: use HEAD as secondary " Greg Hurrell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YL/qFDPoF/0+ArAV@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=greg@hurrell.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.