git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* How to combine multiple commit diffs?
@ 2023-10-12 12:00 ZheNing Hu
  2023-10-12 16:41 ` Johannes Sixt
  2023-10-12 17:03 ` Junio C Hamano
  0 siblings, 2 replies; 5+ messages in thread
From: ZheNing Hu @ 2023-10-12 12:00 UTC (permalink / raw)
  To: Git List, Junio C Hamano

Hi everyone,

Our company wants to design a "small-batch" code review feature.
Simply put, this "small-batch" means being able to treat multiple
related commits within a MergeRequest as an independent "small" code
review.

Let me give you an example: We have five commits: A1, B, A2, C, A3.
Among them, A1, A2, and A3 are multiple commits for the same feature.
So when the user selects these commits, the page will return a
"combine diff" that combines them together.

A1       B A2 A3 C
*--------*----*-----*-------* (branch)
 \ A1'        \ A2'  \ A3'
  *------------*------*------- (small branch code review)

This may seem similar to cherry-picking a few commits from a pile of
commits, but in fact, we do not expect to actually perform
cherry-picking.

Do you have any suggestions on how we can merge a few commits together
and display the diff? The only reference we have is the non-open
source platform, JetBrains Space CodeReview, they support selecting
multiple commits for CodeReview. [1], .

[1]: https://www.jetbrains.com/help/space/review-code.html#code-review-example

Thanks,
--
ZheNing Hu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: How to combine multiple commit diffs?
  2023-10-12 12:00 How to combine multiple commit diffs? ZheNing Hu
@ 2023-10-12 16:41 ` Johannes Sixt
  2023-10-18 12:47   ` ZheNing Hu
  2023-10-12 17:03 ` Junio C Hamano
  1 sibling, 1 reply; 5+ messages in thread
From: Johannes Sixt @ 2023-10-12 16:41 UTC (permalink / raw)
  To: ZheNing Hu; +Cc: Git List, Junio C Hamano

[For general support questions, please address only the mailing list.
It's not necessary to bother the maintainer personally. Though, I'll not
remove him from Cc, yet, as to comply with this ML's etiquette.]

Am 12.10.23 um 14:00 schrieb ZheNing Hu:
> Hi everyone,
> 
> Our company wants to design a "small-batch" code review feature.
> Simply put, this "small-batch" means being able to treat multiple
> related commits within a MergeRequest as an independent "small" code
> review.
> 
> Let me give you an example: We have five commits: A1, B, A2, C, A3.
> Among them, A1, A2, and A3 are multiple commits for the same feature.
> So when the user selects these commits, the page will return a
> "combine diff" that combines them together.
> 
> A1       B A2 A3 C
> *--------*----*-----*-------* (branch)
>  \ A1'        \ A2'  \ A3'
>   *------------*------*------- (small branch code review)
> 
> This may seem similar to cherry-picking a few commits from a pile of
> commits, but in fact, we do not expect to actually perform
> cherry-picking.
> 
> Do you have any suggestions on how we can merge a few commits together
> and display the diff? The only reference we have is the non-open
> source platform, JetBrains Space CodeReview, they support selecting
> multiple commits for CodeReview. [1], .


Take a step back. Then ask: What are the consequences of the review?
What if the result is: the feature is perfect, we want it merged,
however, we cannot, because we do not want commit B. What if the result
is the opposite? You need B, but you can't merge it because the feature
is not ready, yet?

You are looking for a technical workaround for a non-optimal workflow.
If A1,A2,A3 are a feature on their own, they, and only they, should be
in their own feature branch.

So, I would say, the best solution is to reorder the commits in a better
manageable order. You do know about git rebase --interactive, don't you?

-- Hannes


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: How to combine multiple commit diffs?
  2023-10-12 12:00 How to combine multiple commit diffs? ZheNing Hu
  2023-10-12 16:41 ` Johannes Sixt
@ 2023-10-12 17:03 ` Junio C Hamano
  2023-10-18 13:05   ` ZheNing Hu
  1 sibling, 1 reply; 5+ messages in thread
From: Junio C Hamano @ 2023-10-12 17:03 UTC (permalink / raw)
  To: ZheNing Hu; +Cc: Git List

ZheNing Hu <adlternative@gmail.com> writes:

> This may seem similar to cherry-picking a few commits from a pile of
> commits, but in fact, we do not expect to actually perform
> cherry-picking.

If you said "We do not want to", I may be sympathetic, but it is
curious that you said "We do not expect to", which hints that you
already have some implementation in mind.

Whether you use a checkout of the history to a working tree and
perform a series of "git cherry-pick" to implement it, or use the
"git replay" machinery to perform them in-core, at the conceptual
level, you would need to pick a base commit and apply the effect of
those commits you would want to look at on top of that base in
sequence, before you can see the combined change, no?

Puzzled.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: How to combine multiple commit diffs?
  2023-10-12 16:41 ` Johannes Sixt
@ 2023-10-18 12:47   ` ZheNing Hu
  0 siblings, 0 replies; 5+ messages in thread
From: ZheNing Hu @ 2023-10-18 12:47 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Git List, Junio C Hamano

Johannes Sixt <j6t@kdbg.org> 于2023年10月13日周五 00:41写道:
>
> [For general support questions, please address only the mailing list.
> It's not necessary to bother the maintainer personally. Though, I'll not
> remove him from Cc, yet, as to comply with this ML's etiquette.]
>

OK... I get it.

> Am 12.10.23 um 14:00 schrieb ZheNing Hu:
> > Hi everyone,
> >
> > Our company wants to design a "small-batch" code review feature.
> > Simply put, this "small-batch" means being able to treat multiple
> > related commits within a MergeRequest as an independent "small" code
> > review.
> >
> > Let me give you an example: We have five commits: A1, B, A2, C, A3.
> > Among them, A1, A2, and A3 are multiple commits for the same feature.
> > So when the user selects these commits, the page will return a
> > "combine diff" that combines them together.
> >
> > A1       B A2 A3 C
> > *--------*----*-----*-------* (branch)
> >  \ A1'        \ A2'  \ A3'
> >   *------------*------*------- (small branch code review)
> >
> > This may seem similar to cherry-picking a few commits from a pile of
> > commits, but in fact, we do not expect to actually perform
> > cherry-picking.
> >
> > Do you have any suggestions on how we can merge a few commits together
> > and display the diff? The only reference we have is the non-open
> > source platform, JetBrains Space CodeReview, they support selecting
> > multiple commits for CodeReview. [1], .
>
>
> Take a step back. Then ask: What are the consequences of the review?
> What if the result is: the feature is perfect, we want it merged,
> however, we cannot, because we do not want commit B. What if the result
> is the opposite? You need B, but you can't merge it because the feature
> is not ready, yet?
>

The CodeReview here is only expected to review the Diff changes
 (just like jetbrains Space). If it is truly blocked by B, users should
understand to remove B or cherrypick A1, A2, A3 into a new branch.


> You are looking for a technical workaround for a non-optimal workflow.
> If A1,A2,A3 are a feature on their own, they, and only they, should be
> in their own feature branch.
>

I have to admit that this is addressing the issue for users who are not
very familiar with the git workflow, as they might add 70 commits in a
CodeReview.
My goal is to enable these users to display the diff of multiple commits
that they consider to be related together.

> So, I would say, the best solution is to reorder the commits in a better
> manageable order. You do know about git rebase --interactive, don't you?
>

It would be great if users knew how to do that, and I wouldn't have to
explore such unconventional technical solutions :(

> -- Hannes
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: How to combine multiple commit diffs?
  2023-10-12 17:03 ` Junio C Hamano
@ 2023-10-18 13:05   ` ZheNing Hu
  0 siblings, 0 replies; 5+ messages in thread
From: ZheNing Hu @ 2023-10-18 13:05 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List

Junio C Hamano <gitster@pobox.com> 于2023年10月13日周五 01:03写道:
>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> > This may seem similar to cherry-picking a few commits from a pile of
> > commits, but in fact, we do not expect to actually perform
> > cherry-picking.
>
> If you said "We do not want to", I may be sympathetic, but it is
> curious that you said "We do not expect to", which hints that you
> already have some implementation in mind.
>

Our code platform has the ability to cherry-pick (by libgit2),
but using cherry-pick to merge and display the diff of the same feature
might be challenging for users (as it can easily meet conflicts).

> Whether you use a checkout of the history to a working tree and
> perform a series of "git cherry-pick" to implement it, or use the
> "git replay" machinery to perform them in-core, at the conceptual
> level, you would need to pick a base commit and apply the effect of
> those commits you would want to look at on top of that base in
> sequence, before you can see the combined change, no?
>

Yes, "cherry pick" and "rebase -i" are indeed the most natural
approaches to merging and displaying diffs. It's unfortunate
that we didn't choose to use them in our technical selection.

> Puzzled.

I can only roughly estimate how Jetbrains Space solves this problem:

1. The user selects a few non-consecutive commits in the commit list.
2. The backend service will calculate the set of modified files for the selected
    commits in the CodeReview.
3. The backend service it calculates the latest commit and oldest parent commit
    of each file that introduced the diff in the selected commits.
4. it calculates the file diff using these two commits.
5. Afterwards, there is some way to filter out the intermediate, non-selected
   commits that modified the files. I don't quite understand this
step, it feels rather hacker-like.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-10-18 13:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-12 12:00 How to combine multiple commit diffs? ZheNing Hu
2023-10-12 16:41 ` Johannes Sixt
2023-10-18 12:47   ` ZheNing Hu
2023-10-12 17:03 ` Junio C Hamano
2023-10-18 13:05   ` ZheNing Hu

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 NNTP newsgroup(s).