All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Remo Senekowitsch" <remo@buenzli.dev>
To: "Nico Williams" <nico@cryptonector.com>,
	"D. Ben Knoble" <ben.knoble@gmail.com>
Cc: "Theodore Ts'o" <tytso@mit.edu>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Martin von Zweigbergk" <martinvonz@google.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Edwin Kempin" <ekempin@google.com>,
	"Scott Chacon" <scott@gitbutler.com>,
	"philipmetzger@bluewin.ch" <philipmetzger@bluewin.ch>
Subject: Re: Semantics of change IDs (Re: Gerrit, GitButler, and Jujutsu projects collaborating on change-id commit footer)
Date: Wed, 23 Apr 2025 02:25:40 +0200	[thread overview]
Message-ID: <D9DLAQTMJYU6.RJLLVMQZOICK@buenzli.dev> (raw)
In-Reply-To: <aAgWytQNqtLzg2TU@ubby>

On Wed Apr 23, 2025 at 12:23 AM CEST, Nico Williams wrote:
> On Tue, Apr 22, 2025 at 04:17:03PM -0400, D. Ben Knoble wrote:
>> On Wed, Apr 16, 2025 at 7:36 AM Remo Senekowitsch <remo@buenzli.dev> wrote:
>> > On Mon Apr 14, 2025 at 9:54 PM CEST, D. Ben Knoble wrote:
>
> [Responding out of order.]
>
>> > I think it's the other way around: Building a code review UI built on
>> > git and centered around interdiffs today is _hard_, that's why we don't
>> > have it yet. Adding change-ids to commits will make it much easier,
>> > paving the way for these tools to be implemented.
>> 
>> Fair point, although GitHub's detection of force-pushes makes me think
>> it could split a PR into versions at that point, cross-link backwards
>> and forwards by one version (from the force-push detection), show
>> range-diffs between versions based on the target branch of the merge,
>> and even follow the cross-links to show an overall sequence of
>> versions.
>
> GitLab does something like this for review comments where the author
> updates the commented you get a link you can click on that shows you the
> differences between version n-1 and version n.  GitLab does this without
> change IDs.
>
> GitLab seems to figure it out -- an existence proof that it can be done.
> So maybe Junio and Theodore are quite right that similarity checks
> should be enough.

I haven't used GitLab in a while so I had to test it. I found the
feature you describe and it's nice, but it falls short in important and
somewhat predicable ways.

Firstly, it only happens when you make a comment on the whole diff.
Then it shows you an interdiff between the version you commented on and
the immediate next version. (I didn't find a way to make it show the
interdiff between the commented-on and the _latest_ version, but that
seems doable implementation wise.)

But that's not really what we're talking about here. That's taking the
interdiff between two versions _of the entire branch_ and selecting a
relevant _hunk_ from it. Neat, but completely unrelated.

If you make a comment on a _specific commit_, the feature doesn't kick
in at all. GitLab just tells you that this comment was made "on an
outdated change in commit xyz".

A truly patch-based review UI would show the interdiff between the
previous version of the specific commit that was commented on and the
next or latest version of _that specific patch_.

That requires tracking how the individual patches evolve and as far as
I can tell, GitLab makes no attempt to do so. I didn't even give it a
hard time, I kept the number of commits, the commit messages and author
timestamps the same.

So, while I must say GitLab is doing pretty well here, claiming that
they "figured out" patch-based review on top of git without change-ids
is not accurate.

  parent reply	other threads:[~2025-04-23  0:25 UTC|newest]

Thread overview: 118+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-02 18:48 Gerrit, GitButler, and Jujutsu projects collaborating on change-id commit footer Martin von Zweigbergk
2025-04-02 19:34 ` Remo Senekowitsch
2025-04-02 19:49   ` Konstantin Ryabitsev
2025-04-02 19:45 ` Konstantin Ryabitsev
2025-04-02 19:52 ` Martin von Zweigbergk
2025-04-03  9:09 ` Patrick Steinhardt
2025-04-03 10:38   ` Remo Senekowitsch
2025-04-03 11:06     ` Patrick Steinhardt
2025-04-03 15:56   ` Elijah Newren
2025-04-03 16:25     ` Remo Senekowitsch
2025-04-03 16:38       ` Elijah Newren
2025-04-03 21:46         ` Martin von Zweigbergk
2025-04-04  9:41     ` Patrick Steinhardt
2025-04-03 15:39 ` Elijah Newren
2025-04-03 16:40   ` Remo Senekowitsch
2025-04-03 22:11     ` Kane York
2025-04-04  2:28     ` Elijah Newren
2025-04-04  2:40       ` Elijah Newren
2025-04-04  3:47         ` Martin von Zweigbergk
2025-04-04  4:03           ` Nico Williams
2025-04-04  4:59           ` Elijah Newren
2025-04-04  5:21             ` Martin von Zweigbergk
2025-04-04  9:29               ` Patrick Steinhardt
2025-04-03 17:48   ` Theodore Ts'o
2025-04-03 20:31     ` Remo Senekowitsch
2025-04-05  2:09       ` Theodore Ts'o
2025-04-03 18:10   ` Nico Williams
2025-04-03 21:45     ` Remo Senekowitsch
     [not found]       ` <Z+8GoNrdaJlmNpGm@ubby>
2025-04-04  0:05         ` Remo Senekowitsch
2025-04-04  3:52           ` Nico Williams
2025-04-04  7:41             ` Remo Senekowitsch
2025-04-04 16:08               ` Nico Williams
2025-04-03 22:05     ` Martin von Zweigbergk
2025-04-03 22:13       ` Nico Williams
2025-04-03 22:47         ` Martin von Zweigbergk
2025-04-04  2:06           ` Elijah Newren
2025-04-04  3:11           ` Nico Williams
2025-04-04  4:08             ` Martin von Zweigbergk
2025-04-04  4:23               ` Nico Williams
2025-04-04  9:34                 ` Patrick Steinhardt
2025-04-04 16:04                   ` Nico Williams
2025-04-07  8:00                     ` Patrick Steinhardt
2025-04-07 20:59 ` Junio C Hamano
2025-04-07 21:36   ` Nico Williams
2025-04-08 12:55     ` Theodore Ts'o
2025-04-08 15:53       ` Nico Williams
2025-04-09 12:19         ` Theodore Ts'o
2025-04-09 12:56           ` Junio C Hamano
2025-04-09 19:13             ` Nico Williams
2025-04-10  8:29               ` Junio C Hamano
2025-04-10 21:40                 ` Martin von Zweigbergk
2025-04-09 16:54           ` Semantics of change IDs (Re: Gerrit, GitButler, and Jujutsu projects collaborating on change-id commit footer) Nico Williams
2025-04-09 18:02             ` Junio C Hamano
2025-04-09 18:35               ` Nico Williams
2025-04-09 19:14                 ` Eric Sunshine
2025-04-09 19:31                   ` Nico Williams
2025-04-10 13:44                 ` Theodore Ts'o
2025-04-10 16:18                   ` Junio C Hamano
2025-04-11 15:48                     ` Theodore Ts'o
2025-04-11 16:38                       ` Konstantin Ryabitsev
2025-04-11 17:44                       ` Junio C Hamano
2025-04-12 23:13                         ` Theodore Ts'o
2025-04-14 15:13                           ` Junio C Hamano
2025-04-15 22:30                             ` Remo Senekowitsch
2025-04-16  0:09                               ` Junio C Hamano
2025-04-16  0:21                               ` Jacob Keller
2025-04-15 21:38                           ` Jacob Keller
2025-04-14 19:54             ` D. Ben Knoble
2025-04-14 21:34               ` Nico Williams
2025-04-15 21:44               ` Jacob Keller
2025-04-16 11:36               ` Remo Senekowitsch
2025-04-22 20:17                 ` D. Ben Knoble
2025-04-22 22:24                   ` Remo Senekowitsch
2025-04-22 22:42                     ` Junio C Hamano
2025-04-22 22:51                       ` Nico Williams
2025-04-22 23:47                         ` Remo Senekowitsch
2025-04-23  0:32                           ` Nico Williams
2025-04-23  1:15                             ` Remo Senekowitsch
2025-04-23  4:45                               ` Nico Williams
2025-04-22 23:49                         ` Junio C Hamano
2025-04-23  1:02                           ` Nico Williams
2025-04-23  4:47                             ` Nico Williams
2025-04-22 23:21                       ` Remo Senekowitsch
2025-04-23  5:07                       ` Martin von Zweigbergk
2025-04-23 15:51                         ` Junio C Hamano
2025-04-23 16:19                           ` Martin von Zweigbergk
2025-06-06 13:04                             ` Toon Claes
     [not found]                   ` <aAgWytQNqtLzg2TU@ubby>
2025-04-23  0:25                     ` Remo Senekowitsch [this message]
2025-04-23  0:45                       ` Nico Williams
2025-04-23 12:58                         ` How GitLab does/doesn't need change IDs (was Re: Semantics of change IDs) Toon Claes
2025-04-23 18:59                           ` Nico Williams
2025-05-10 19:32                     ` Semantics of change IDs (Re: Gerrit, GitButler, and Jujutsu projects collaborating on change-id commit footer) D. Ben Knoble
2025-05-10 19:46                       ` D. Ben Knoble
2025-05-10 20:31                         ` Martin von Zweigbergk
2025-05-12 17:03                           ` Junio C Hamano
2025-05-12 17:19                             ` Martin von Zweigbergk
2025-05-14 14:38                               ` Junio C Hamano
2025-05-15 10:31                                 ` Oswald Buddenhagen
2025-05-15 16:32                                   ` Jacob Keller
2025-05-15 19:59                                     ` Junio C Hamano
2025-05-15 20:10                                       ` Nico Williams
     [not found]                           ` <aCJi+4q6DZhnfdy+@ubby>
2025-05-12 21:43                             ` Martin von Zweigbergk
2025-05-12 22:04                               ` brian m. carlson
2025-06-06 12:28                                 ` Toon Claes
2025-06-06 15:44                                   ` Junio C Hamano
2025-05-13 21:22                               ` D. Ben Knoble
2025-04-07 22:51   ` Gerrit, GitButler, and Jujutsu projects collaborating on change-id commit footer Remo Senekowitsch
2025-04-08  0:10   ` Junio C Hamano
2025-04-08  5:35     ` Martin von Zweigbergk
2025-04-08 14:27       ` Junio C Hamano
2025-04-08 15:58         ` Phillip Wood
2025-04-08 16:27           ` Nico Williams
2025-04-12 21:32           ` Junio C Hamano
2025-04-16  0:24         ` Jacob Keller
2025-05-14 15:08         ` Kristoffer Haugsbakk
2025-04-08 14:27       ` Junio C Hamano
2025-08-19 14:04 ` Askar Safin
2025-08-19 16:44   ` Ben Knoble

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=D9DLAQTMJYU6.RJLLVMQZOICK@buenzli.dev \
    --to=remo@buenzli.dev \
    --cc=ben.knoble@gmail.com \
    --cc=ekempin@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martinvonz@google.com \
    --cc=nico@cryptonector.com \
    --cc=philipmetzger@bluewin.ch \
    --cc=scott@gitbutler.com \
    --cc=tytso@mit.edu \
    /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.