jensroemer.com
oppi.li
+1
-1
input.css
+1
-1
input.css
+364
pages/blog/stacking.md
+364
pages/blog/stacking.md
···
1
+
---
2
+
atroot: true
3
+
template:
4
+
slug: stacking
5
+
title: jujutsu-style code review
6
+
subtitle: we now support jujutsu style "interdiff" code-review!
7
+
date: 2025-06-02
8
+
image: /static/img/hidden-ref.png
9
+
authors:
10
+
- name: Akshay
11
+
email: akshay@tangled.sh
12
+
handle: oppi.li
13
+
draft: true
14
+
---
15
+
16
+
Jujutsu is built around structuring your work into
17
+
meaningful commits. Naturally, during code-review, you'd
18
+
expect reviewers to be able to comment on individual
19
+
commits, and also see the evolution of a commit over time,
20
+
as reviews are addressed. We set out to natively support
21
+
this model of code-review on Tangled.
22
+
23
+
If you're new to Tangled, [read our intro](/intro) for more
24
+
about the project.
25
+
26
+
For starters, I would like to contrast the two schools of
27
+
code-review, the "diff-soup" model and the interdiff model.
28
+
29
+
## the diff soup model
30
+
31
+
When you create a PR on traditional code-forges (GitHub
32
+
specifically), the UX implicitly encourages you to address
33
+
your code review by *adding commits* on top of your original
34
+
set of changes:
35
+
36
+
- GitHub's "Apply Suggestion" button directly commits the
37
+
suggestion into your PR
38
+
- GitHub only shows you the diff of all files at once by
39
+
default
40
+
- It is difficult to know what changed across force-pushes
41
+
42
+
Consider a hypothetical PR that adds 3 commits:
43
+
44
+
```
45
+
[c] implement new feature across the board (HEAD)
46
+
|
47
+
[b] introduce new feature
48
+
|
49
+
[a] some small refactor
50
+
```
51
+
52
+
And when only *newly added commits* are easy to review, this
53
+
is what ends up happening:
54
+
55
+
```
56
+
[f] formatting & linting (HEAD)
57
+
|
58
+
[e] update name of new feature
59
+
|
60
+
[d] fix bug in refactor
61
+
|
62
+
[c] implement new feature across the board
63
+
|
64
+
[b] introduce new feature
65
+
|
66
+
[a] some small refactor
67
+
```
68
+
69
+
It is impossible to tell what addresses what at a glance,
70
+
there is an implicit relation between each change:
71
+
72
+
```
73
+
[f] formatting & linting
74
+
|
75
+
[e] update name of new feature -------------.
76
+
| |
77
+
[d] fix bug in refactor -----------. |
78
+
| | |
79
+
[c] implement new feature across the board |
80
+
| | |
81
+
[b] introduce new feature <-----------------'
82
+
| |
83
+
[a] some small refactor <----------'
84
+
```
85
+
86
+
This has the downside of clobbering the output of `git
87
+
blame` (if there is a bug in the new feature, you will first
88
+
land on `e`, and upon digging further, you will land on
89
+
`b`).
90
+
91
+
92
+
## the interdiff model
93
+
94
+
With jujutsu however, you have the tools at hand to
95
+
fearlessly edit, split, squash and rework old commits (you
96
+
could do this with git if you are familiar with fixup
97
+
commits or interactive rebasing).
98
+
99
+
Lets try that again:
100
+
101
+
```
102
+
[c] implement new feature across the board (HEAD)
103
+
|
104
+
[b] introduce new feature
105
+
|
106
+
[a] some small refactor
107
+
```
108
+
109
+
To fix the bug in the refactor:
110
+
111
+
```
112
+
$ jj edit a
113
+
Working copy (@) now at: [a] some small refactor
114
+
115
+
$ # hack hack hack
116
+
117
+
$ jj log -r a::
118
+
Rebased 2 descendant commits onto updated working copy
119
+
[c] implement new feature across the board (HEAD)
120
+
|
121
+
[b] introduce new feature
122
+
|
123
+
[a] some small refactor
124
+
```
125
+
126
+
This is the most important part:
127
+
128
+
```
129
+
Rebased 2 descendant commits onto updated working copy
130
+
```
131
+
132
+
Jujutsu automatically rebases the descendants without having
133
+
to lift a finger. Brilliant! You can repeat the same
134
+
exercise for all review comments, and effectively, your
135
+
PR will have evolved like so:
136
+
137
+
```
138
+
a -> b -> c
139
+
| | |
140
+
v v v
141
+
a' -> b' -> c'
142
+
```
143
+
144
+
## the catch
145
+
146
+
If you use `git rebase`, you will know that it modifies
147
+
history and therefore changes the commit SHA. How then,
148
+
should one tell the difference between the "old" and "new"
149
+
state of affairs?
150
+
151
+
Tools like `git-range-diff` make use of a variety of
152
+
text-based heuristics to roughly match `a` to `a'` and `b`
153
+
to `b'` etc.
154
+
155
+
Jujutsu however, works around this by assigning stable
156
+
"change id"s to each change (which internally point to a git
157
+
commit, if you use the git backing). If you edit an old
158
+
commit, its SHA changes, but its change-id remains the same.
159
+
160
+
And this is the essence of our new stacked PRs feature!
161
+
162
+
## interdiff code review on tangled
163
+
164
+
To really explain how this works, lets start with a [new
165
+
codebase](https://tangled.sh/@oppi.li/stacking-demo/):
166
+
167
+
```
168
+
$ jj git init --colocate
169
+
170
+
# -- initialize codebase --
171
+
172
+
$ jj log
173
+
@ n set: introduce Set type main HEAD 1h
174
+
```
175
+
176
+
I have kicked things off by creating a new go module that
177
+
adds a `HashSet` data structure. My first changeset
178
+
introduces some basic set operations:
179
+
180
+
```
181
+
$ jj log
182
+
@ so set: introduce set difference HEAD
183
+
├ sq set: introduce set intersection
184
+
├ mk set: introduce set union
185
+
├ my set: introduce basic set operations
186
+
~
187
+
188
+
$ jj git push -c @
189
+
Changes to push to origin:
190
+
Add bookmark push-soqmukrvport to fc06362295bd
191
+
```
192
+
193
+
On the New Pull Request page, when submitting this branch
194
+
for review, select "Submit as stacked PRs":
195
+
196
+
<figure class="max-w-[550px] m-auto flex flex-col items-center justify-center">
197
+
<a href="static/img/submit_stacked.jpeg">
198
+
<img class="my-1 h-auto max-w-full" src="static/img/submit_stacked.jpeg">
199
+
</a>
200
+
<figcaption class="text-center">Submitting Stacked PRs</figcaption>
201
+
</figure>
202
+
203
+
This submits each change as an individual pull request:
204
+
205
+
<figure class="max-w-[550px] m-auto flex flex-col items-center justify-center">
206
+
<a href="static/img/top_of_stack.jpeg">
207
+
<img class="my-1 h-auto max-w-full" src="static/img/top_of_stack.jpeg">
208
+
</a>
209
+
<figcaption class="text-center">The "stack" attachment is similar to Gerrit's relation chain</figcaption>
210
+
</figure>
211
+
212
+
After a while, I receive a couple of review comments, but
213
+
not on my entire submission, but rather, on each *individual
214
+
change*. Additionally, the reviewer is happy with my first
215
+
change, and has gone ahead and merged that:
216
+
217
+
<div class="flex justify-center items-start gap-2">
218
+
<figure class="w-1/3 m-0 flex flex-col items-center">
219
+
<a href="static/img/basic_merged.jpeg">
220
+
<img class="my-1 w-full h-auto cursor-pointer" src="static/img/basic_merged.jpeg" alt="The first change has been merged">
221
+
</a>
222
+
<figcaption class="text-center">The first change has been merged</figcaption>
223
+
</figure>
224
+
225
+
<figure class="w-1/3 m-0 flex flex-col items-center">
226
+
<a href="static/img/review_union.jpeg">
227
+
<img class="my-1 w-full h-auto cursor-pointer" src="static/img/review_union.jpeg" alt="A review on the set union implementation">
228
+
</a>
229
+
<figcaption class="text-center">A review on the set union implementation</figcaption>
230
+
</figure>
231
+
232
+
<figure class="w-1/3 m-0 flex flex-col items-center">
233
+
<a href="static/img/review_difference.jpeg">
234
+
<img class="my-1 w-full h-auto cursor-pointer" src="static/img/review_difference.jpeg" alt="A review on the set difference implementation">
235
+
</a>
236
+
<figcaption class="text-center">A review on the set difference implementation</figcaption>
237
+
</figure>
238
+
</div>
239
+
240
+
Let us address the first review:
241
+
242
+
> can you use the new `maps.Copy` api here?
243
+
244
+
```
245
+
$ jj log
246
+
@ so set: introduce set difference push-soqmukrvport
247
+
├ sq set: introduce set intersection
248
+
├ mk set: introduce set union
249
+
├ my set: introduce basic set operations
250
+
~
251
+
252
+
# lets edit the implementation of `Union`
253
+
$ jj edit mk
254
+
255
+
# hack, hack, hack
256
+
257
+
$ jj log
258
+
Rebased 2 descendant commits onto updated working copy
259
+
├ so set: introduce set difference push-soqmukrvport*
260
+
├ sq set: introduce set intersection
261
+
@ mk set: introduce set union
262
+
├ my set: introduce basic set operations
263
+
~
264
+
```
265
+
266
+
Next, let us address the bug:
267
+
268
+
> there is a logic bug here, the condition should be negated.
269
+
270
+
```
271
+
# lets edit the implementation of `Difference`
272
+
$ jj edit so
273
+
274
+
# hack, hack, hack
275
+
276
+
$ jj log
277
+
@ so set: introduce set difference push-soqmukrvport*
278
+
├ sq set: introduce set intersection
279
+
├ mk set: introduce set union
280
+
├ my set: introduce basic set operations
281
+
~
282
+
```
283
+
284
+
We are done addressing reviews, let us push our code:
285
+
```
286
+
$ jj git push
287
+
Changes to push to origin:
288
+
Move sideways bookmark push-soqmukrvport from fc06362295bd to dfe2750f6d40
289
+
```
290
+
291
+
Upon resubmitting the PR for review:
292
+
293
+
<div class="flex justify-center items-start gap-2">
294
+
<figure class="w-1/2 m-0 flex flex-col items-center">
295
+
<a href="static/img/round_2_union.jpeg">
296
+
<img class="my-1 w-full h-auto" src="static/img/round_2_union.jpeg" alt="PR #2 advances to the next round">
297
+
</a>
298
+
<figcaption class="text-center">PR #2 advances to the next round</figcaption>
299
+
</figure>
300
+
301
+
<figure class="w-1/2 m-0 flex flex-col items-center">
302
+
<a href="static/img/round_2_difference.jpeg">
303
+
<img class="my-1 w-full h-auto" src="static/img/round_2_difference.jpeg" alt="PR #4 advances to the next round">
304
+
</a>
305
+
<figcaption class="text-center">PR #4 advances to the next round</figcaption>
306
+
</figure>
307
+
</div>
308
+
309
+
Of note here are a few things:
310
+
311
+
- The initial submission is still visible under `round #0`
312
+
- By resubmitting, the round has simply advanced to `round
313
+
#1`
314
+
- There is a helpful "interdiff" button to look at the
315
+
difference between the two submissions
316
+
317
+
The individual diffs are still available, but most
318
+
importantly, the reviewer can view the *evolution* of a
319
+
change by hitting the interdiff button:
320
+
321
+
<div class="flex justify-center items-start gap-2">
322
+
<figure class="w-1/2 m-0 flex flex-col items-center">
323
+
<a href="static/img/diff_1_difference.jpeg">
324
+
<img class="my-1 w-full h-auto" src="static/img/diff_1_difference.jpeg" alt="Diff from round #0">
325
+
</a>
326
+
<figcaption class="text-center">Diff from round #0</figcaption>
327
+
</figure>
328
+
329
+
<figure class="w-1/2 m-0 flex flex-col items-center">
330
+
<a href="static/img/diff_2_difference.jpeg">
331
+
<img class="my-1 w-full h-auto" src="static/img/diff_2_difference.jpeg" alt="Diff from round #1">
332
+
</a>
333
+
<figcaption class="text-center">Diff from round #1</figcaption>
334
+
</figure>
335
+
</div>
336
+
337
+
<figure class="max-w-[550px] m-auto flex flex-col items-center justify-center">
338
+
<a href="static/img/interdiff_difference.jpeg">
339
+
<img class="my-1 w-full h-auto" src="static/img/interdiff_difference.jpeg" alt="Interdiff between round #0 and #1">
340
+
</a>
341
+
<figcaption class="text-center">Interdiff between round #1 and #0</figcaption>
342
+
</figure>
343
+
344
+
Indeed, the logic bug has been addressed!
345
+
346
+
## start stacking today
347
+
348
+
If you are a jujutsu user, you can enable this flag on more
349
+
recent versions of jujutsu:
350
+
351
+
```
352
+
λ jj --version
353
+
jj 0.29.0-8c7ca30074767257d75e3842581b61e764d022cf
354
+
355
+
# -- in your config.toml file --
356
+
[git]
357
+
write-change-id-header = true
358
+
```
359
+
360
+
This feature writes `change-id` headers directly into the
361
+
git commit object, and is visible to code-forges upon push,
362
+
and allows you to stack your PRs on Tangled.
363
+
364
+
static/img/basic_merged.jpeg
static/img/basic_merged.jpeg
This is a binary file and will not be displayed.
static/img/diff_1_difference.jpeg
static/img/diff_1_difference.jpeg
This is a binary file and will not be displayed.
static/img/diff_1_union.jpeg
static/img/diff_1_union.jpeg
This is a binary file and will not be displayed.
static/img/diff_2_difference.jpeg
static/img/diff_2_difference.jpeg
This is a binary file and will not be displayed.
static/img/diff_2_union.jpeg
static/img/diff_2_union.jpeg
This is a binary file and will not be displayed.
static/img/interdiff_difference.jpeg
static/img/interdiff_difference.jpeg
This is a binary file and will not be displayed.
static/img/interdiff_union.jpeg
static/img/interdiff_union.jpeg
This is a binary file and will not be displayed.
static/img/review_difference.jpeg
static/img/review_difference.jpeg
This is a binary file and will not be displayed.
static/img/review_union.jpeg
static/img/review_union.jpeg
This is a binary file and will not be displayed.
static/img/round_2_difference.jpeg
static/img/round_2_difference.jpeg
This is a binary file and will not be displayed.
static/img/round_2_union.jpeg
static/img/round_2_union.jpeg
This is a binary file and will not be displayed.
static/img/submit_stacked.jpeg
static/img/submit_stacked.jpeg
This is a binary file and will not be displayed.
static/img/top_of_stack.jpeg
static/img/top_of_stack.jpeg
This is a binary file and will not be displayed.