Tomas Vondra

Tomas Vondra

blog about Postgres code and community

Being considerate of other people's time

If I could give one piece of advice to the past me, starting to contribute to open source projects, it’d be to value other developers' time more. It took me a while to appreciate the “economy” behind this, and adjust how I work to increase my chance of getting patches done. Hopefully some new contributors could learn from my mistakes.

My experience is that most people in the Postgres developer community want to help others - review and test patches, etc. Everyone has only a limited amount of time for this, though. People are working on their own patches too, possibly only in their free time, etc.

There’ll always be more patches to review than available reviewers, just like TODO lists tend to have more items than what can be actually done. And that’s fine, it forces us to prioritize and focus on the most useful and feasible improvements. It also means there’s a continuous arbitrage - reviewers pick what to review next and what to skip.

How do people decide which patches to review?

Sure, the topic of a patch matters. But there’s usually multiple patches from the same area. The next question is “Is this an efficient use of my time?” It’s worth considering that before posting a patch to the list, and focus on making it an efficient choice for other people.

Let me give you a couple examples of how I think about this - as a patch author, reviewer and a committer. I don’t have the “Do this one thing …” kind of click-baity advice. It’s more of a general principle, and you’ll need to figure out how to apply it to various situations.

There’s a lot of nuance, of course. It’s not a crisp black-or-white kind of rule, more of a trade off situation.

If you can do more, you probably should

Let’s say you’re working on a patch, and you want to post it to the list already. There’s one more change needed, to fix a design issue. It’d take ~10 minutes of your time, but it’s mostly mechanical, anyone can do it. So you post the patch without it, to get the feedback earlier.

This unfortunately has two likely consequences. Some reviewers will point out that one change - which you already planned to do, so it’s not very useful feedback. And other reviewers will end up doing the change themselves, and now you’ve wasted 10 minutes of each reviewer’s time.

If you can save time for other people, do it. It may be just 10 minutes of your time, but with multiple people it quickly adds up and it can be 1 hour of “community time.”

Reviews are one of the big bottlenecks of our project, so wasting that capacity is especially painful.

This is not just about authoring patches. Maybe you ran some static analyzer tool, and it spit out a bunch of suspected issues. You may either post all those findings to the list verbatim, or spend some of your time investigating which of the findings are valid. We know these tools tend to produce a lot of false positives, so posting all of that to the list forces multiple people to investigate non-issues.

Experimental and PoC/WIP patches

Wait! Does that mean you should not be sending experimental or PoC/WIP patches? Those are by definition incomplete in various ways, right?

Not at all. The earlier section was about work you could have done, but chose not to to save your time. With experimental / WIP patches you’re probably in a situation when you need the feedback to decide what to do next. You can’t proceed without it, you need the feedback first.

It’s perfectly fine to share experimental patches, demonstrating new proposed features. Either to get some feedback regarding the approach, or even the usefulness of the desirability itself. I find it way more productive to have a half-working patch than just a description of what a feature might do.

How to not waste others’ time in this case? For starters, it’s good to label the patch as PoC / WIP. Those patched need a very different type of a review - much higher-level, more about the direction, possible traps etc. It’s annoying to start with a “normal review” only to realize it’s an experimental patch half-way through.

It’s also good to keep track of the “gaps” in experimental patches. By that I mean places / cases that would need additional handling, but you skipped that to get the patch working for a limited set of queries. It’s good to indicate you considered those cases and are aware of the gaps, so that all the reviewers don’t point it out.

A simple XXX or FIXME comment should suffice, or you can add a new README file with a section about the gaps. I prefer the comments, as those go to the relevant place in code.

It also helps to explicitly state what high-level questions you need answering. Should the feature behave like this or that? What is the best way to handle this combination of features? Non-obvious questions reviewers might not ask themselves.

Experimental patches are fine, just label them in some way, track the (known) gaps and state what questions you seek feedback on. Conversely, make it clear when you think a patch is “done” and ready for a pre-commit scrutiny.

Huge patches suck

I’ve seen a bunch of large patches, in the 100KB-500KB range. I’ve even seen a 1MB patch recently. It’s not something I can meaningfully review.

A patch of that size inevitably touches so much code and areas I’d need at least a week of very focused work to form an informed opinion. But am I going to invest a week of my time into a single patch? Unlikely.

The patch might get stuck or abandoned. Or maybe I’ll be asked to do something urgent half-way through the review, before I produce any useful review. Having a week of time dedicated to a patch is not something I have very often. I know that, and it’s unlikely I’ll take the risk.

The best way to mitigate this is to break patches into smaller pieces.

Ideally, there’s a way to develop the feature incrementally, which naturally divides the patch into smaller pieces. The initial pieces may add some necessary infrastructure. Then a piece with a “minimum viable” feature, with various limitations. And then pieces gradually relaxing those limitations. In the end, you get the “full” feature.

Maybe the patch can’t be split like that. I find this to be very rare, more often it’s just about not finding the right split yet. But it’s still better to split the patch in some “artificial” way, e.g. by area of the changes (e.g. catalog, executor, grammar, docs, …).

With a patch split into pieces I can choose to review just one of them. Maybe there is a piece touching an area I’m already familiar with? That should require much less time, with less risk of being distracted. If I get interrupted, I can share at least some meaningful reviews.

As a committer, I strongly prefer the “incremental development” split.

Other committers may be braver, but the chances of me committing a huge (100KB+) patch in one go are rather slim. I’d have to review and understand it enough before committing it. And if I have argued it’s unlikely to have the time for a regular review, it’s even less likely for the thorough pre-commit review.

With incremental pieces, we can cleanup and polish the initial pieces, commit those, and then continue with the following piece. Maybe that happens in the next release, but the first part is already done and users get at least some benefit.

Don’t require reviewers and committers to dedicate huge contiguous chunks of their time to huge patches. It makes review much less likely and commit borderline impossible. Break patches into bite-sized chunks.

Conclusions

I could probably come up with a couple more examples, but I hope the general principle is fairly clear by now. Don’t expect other people to invest significant amounts of time to do something you could have done, and reduce the risk of wasting their time.

I do expect the AI to exacerbate this effect significantly. It makes it so much easier to produce large volumes of “plausible output” - be it patches, reviews, or something else. The demand for good reviews will only increase. Vibe-coded patches are even worse, because in most cases the patch author can’t even answer questions.

It doesn’t need to be perfect, but if you consistently waste other people’s time, they’ll quickly realize it. People will move your patches to the end of their personal TODO, or maybe even start ignoring them.

Do you have feedback on this post? Please reach out by e-mail to tomas@vondra.me.