The Gerrit code review iceberg, episode 2 | Haiku Project

Blog
1

Recently some discussions on the forum led to asking about the status of our Gerrit code review. There are a lot of changes there that have been inactive for several years, with no apparent interest from anyone. To be precise, there are currently 353 commits waiting for review (note that Gerrit, unlike Github and other popular code review tools, works on a commit-by-commit basis, so each commit from a multiple-commit change is counted separately). The oldest one has not seen any comments since 2018.

This is a companion discussion topic for the original entry at https://www.haiku-os.org/blog/pulkomandy/2025-12-05-the_gerrit_pending_review_iceberg_2
12 Likes
2

Thanks @PulkoMandy , this series of articles is indeed very interesting.

3

I find this series of articles interesting too, thanks !

4

@PulkoMandy genuine question, what are the advantages of keeping the abandoned changes in an open state instead of actually marking them as abandoned in Gerrit UI? (before anyone asks, they can be restored afterwards if needed).

When I say abandoned changes, I mean:

The work is unfinished, and completely broken. A later, independant attempt already exists in a more recent change, but is also not merged.

A few questions were asked during code review, but the original author of the changes never replied.
No one has picked up the PowerPC 64 port who could answer these questions. [it’s been 5+ years already].

2 Likes
5

We usually abandon changes when we don’t actually want them (as in we found a different better aproach, the change is not needed anymore etc)

If the change remains open it can actually be found and cleaned up by someone else.

6

Nice to have link to patches in the article but, wouldn’t it be interesting to also have the opposite?
I mean a link to the article as a comment to each patch reviewed. Perhaps done by a new user to avoid the comment to be grouped, and if it is possible, named something like ‘Status end 2025’.

7

Hello,

Good question!

I think there are many answers to this.

First of all, what is the goal? I see a few usages of Gerrit:

As a contributor willing to work on some topic

For me, the goal of Gerrit is to keep track of all proposed changes and things that need to get done.

For example, let’s say I (or some other person) wake up one saturday morning and decide “hey, let’s have a look at porting Haiku to PowerPC64!”. What I will do is:

In a few seconds, I get a good overview of what the status is, and several ways I can help progress in that area.

Now, what if inactive tickets and core review patches were closed? We can have a look at what it would look like with just some tweaks to the previous URLs. I get:

Now, for each abandoned change, I would have to do a mini-investigation: was this abandoned because there is nothing of value, or simply because the original author was busy elsewhere for a few years?

And it would be unusual to look at abandoned changes anyway (just like looking at closed tickets). We tend to assume that they were abandoned because they have no value anymore. These inactive changes are work in progress towards something, and as long as the end goal isn’t reached, they do have some value, even if it requires some further work to extract that value. Maybe an iceberg was not the right image after all. It is more like a gold mine, where you have to keep track of which parts of the land you have already processed and extracted all the gold (these are the abandoned changes), the gold you already extracted (these are the already merged changes), and the parts of the mine still to explore (these are the work in progress changes).

As someone who sent in a patch

Imagine the scenario where you spent some time working on a patch, and submitted it to Gerrit. Then, for whatever reason, you couldn’t get it in a mergeable state. Maybe it exposed a way larger rework to other subsystems is needed. Maybe what you thought would be a simple fix turned out to be a lot more tricky. Or maybe people reviewing your work are not yet ready for such a disruptive change yet. Or maybe you have changing interests and decided to work on something else for a while.

In any case, the change ends up sinking way down in the list and is inactive. After a few years, let’s imagine you get a notification mail saying “No activity for a while, abandoning”. How would you feel about that? When this happens to me in other projects than Haiku, I feel not very good about it. I put some work into that change, and then someone decided to just close the subject, because I was not fast enough to reply to questions, or even worse, because they didn’t review my change quick enough.

As a measurement of progress

Just like the number of open tickets, the number of open changes is an interesting measure. It tells us how good we are at integrating external contributor’s and also our own changes. Surely, having a lot of open changes is not a good thing?

But here, I think you fell victin to Goodhear’s law Goodhart's law - Wikipedia

In other words, if your proposed fix is to just abandon inactive changes, sure, you will get the number of pending reviews down. But you will not actually improve the situation: there will not be any more code being reviewed and changes being completed. You just found a way to game the metric. Sure, technically, the goal “having less open reviews” is reached. But that goal wasn’t the right one. What we want is to eventually get these changes in a state where we can merge them. After all, they all attempt to solve existing problems, even if the solution taken isn’t always the right one.

In a similar spirit, we could suggest bulk moving tickets out of the R1 milestone to make the R1 release happen sooner. This makes sense on the surface: the metric for progress towards R1 is the numbr of tickets left to solve in the roadmap. But that is only a metric. The true meaning is that the system has most bugs resolved, and we are confident in shipping a version ready to take over the desktop computer market. In that view, bulk moving tickets out is just shifting the goalposts (well, in the reverse of how that expression is usually used, but you get the idea).

I don’t think my quick summary of the discussion in these changes has so much value that it needs to be linked back. When I have some really useful insight on what needs to be done, I apply it myself (as happened for the change to BTab label, that I got finished and merged last week).

I also suspect that these articles will get more interesting as we get into upper, warmer layers where things are happening. Eventually, this may turn into a “what’s cooking” newsletter of some sort, who knows?

3 Likes
8

Look at it like someone new to Haiku checking Gerrit for the first time. Your small description adds an historical insight and presents things in layman’s terms. Adding a comment would also mean “We don’t forget about that idea.” and that has value in itself.

9

Thank you for your reviews on these long time not forged codes –
Let’s have new interest awakened in some (new) contributors to have a toss on promising features like

new filesystem write capabilities or CPU environments supports.

10

This is my experience and why I think some patches are stale.

Some of us, are just casual contributors. Someday, something doesn’t work for us and we decide to get into it. Sometimes are easy patches, a missing line, a missing vendor id…

Then when it gets to review appears the negative votes: “There are style issues.“, “This can be refactored“, “This driver has no sense here or there…“

I dont care! I mean, those hundreds of lines of code were already there, I only changed a pair of them. I cannot refactor a code is not mine, I barely understand and already works. It has no sense at all.

I have had also the experience of a patch over an ugly driver and the discussion ends proposing a completely new approach. A casual contributor shouldn’t face architectural changes.

There are other factors, for example Gerrit being hard to understand. Or broken development environment (ie: can’t boot on real hardware anymore).

5 Likes
11

I don’t remember seeing one where adding a vendor ID ended up in such a discussion. But I don’t remember everything, which is one reason for this blog series.

Usually, asking for refactor would happen when the patch is a bit larger (and even then, it is still unreasonable in some cases). The balance between asking too much of new contributors, and making sure the code remains clean and well understood by the maintainers is difficult to find. The other extreme would be “we accepted a lot of casual contributions to this driver, now it is full of hacks, no one understand how it works, and changing it to fix one piece of hardware will break another”.

The idea with keeping the changes on Gerrit would be that someone else can then pick one of these changes and finish up the work. I do it from time to time on parts of Haiku I feel confident about. I hope more people would do it, even some casual contributors.

1 Like
12

Would it be possible to mark changes that are free to take som how or remove owner? I have looked at some changes but don’t want to step on any toes.

2 Likes
13

The other extreme would be “we accepted a lot of casual contributions to this driver, now it is full of hacks

But those drivers were already there, I guess at some point, the driver was introduced as is, because at least, it worked.

I don’t remember seeing one where adding a vendor ID ended up in such a discussion

Maybe not this exact case, but quite similar. Another vendor ID, another case in a switch block, another missing bit. And then the patch gets stuck because there are style issues and some refactoring of code (not belonging or related) to the patch, is suggested.

I think sometimes we ask too much for a contributor, in the sense we ask them to take design decisions they aren’t able to because they don’t have a wide view of all the project.

Also, I want to remark the point of having a good working environment is still difficult these days. So on my personal case, sometimes I have to use another desktop/laptop and haiku no longer boots there.

The idea with keeping the changes on Gerrit would be that someone else can then pick one of these changes and finish up the work

Yes, I see the point of Gerrit, but It is a hard to use tool (Git is, indeed)

I understand It is a difficult problem, however.

14

I don’t think we do (or should) ask for style changes in code you did not touch, maybe small things like if you modified an if conditional you cab format it, but not other functions or sich

1 Like
15

I think anything that is not in page one or two can be considered inactive. And even in these changes, if there were no replies for a few days I tend to assume that the original submitter will be happy that I take over and finish up things.

I don’t think I have annoyed people too much by doing so (hopefully :slight_smile: )

6 Likes