git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / 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

  List information: http://vger.kernel.org/majordomo-info.html

* 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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).