Arnt Gulbrandsen
About meAbout this blog

Code review times two

I like code review but complain about it all the time. Why? The other day I had an eureka moment: There are two quite different kinds, which I will give the misleading names agile review and waterfall review, and I'm annoyed when the reviewers perform one kind of review and I want the other.

Waterfall development is centered around planning, development and deployment in stages: First plan well and get rid of most bugs, then implement what's planned, then review and test to fix the remaining bugs, then deploy. In this model, code review focuses on catching minor bugs, ensuring code style harmony, proper use of internal/thirdparty APIs, etc. Big questions such as is this feature a good idea at all? are out of scope, because that's been settled in the planning phase. (I'm not saying waterfall works in practice, only describing the theory.)

Agile development is different. Instead of planning well, agile development is based around an iterative process: Implement something that seems to be a good idea, consider it, learn something from it, finish it and then deploy, then pick something else to do, something one now knows more about. Code review can be the first part of the consideration and learning process, so it's a good time to discover that although a feature seemed like a good idea, it's going to be a morass and should be dropped at once.

An example may show this better. Consider a server that serves requests from clients and uses a sharded database backend, and a customer who complains that although performance is fine in general, a few requests are much too slow. The reason for this hypothetical problem is that combining results from different shards is too slow.

In an agile process, the team might open an issue saying Some requests are too slow, see support ticket 123456 and bring 95th percentile response time closer to 50th. The developer investigates, write some code, see that it works on the happy path, and ask for review. In this case, the first thing to discuss during code review is what problem is solved by the change and whether it is the ultimate problem or more of a bandaid. Next, perhaps, how to test this approach. Only after that does it make sense to write the tests, implement the sad paths, review the code style, level of testing and other implementation details.

In a waterfall process, one person does the investigation and opens an issue saying something more like access database shards in parallel so that performance is better when many shards are needed for one request, and the questions for code review is whether the implementation conforms to the style guide, whether the tests cover the problem and will avoid false positives/negatives, etc.

These two kinds of review discuss different questions, and they even work best on different code. An agile review should start much earlier than a waterfall review: If you're going to discuss whether you've understood the bug at all, then it makes sense to discuss that before you bother with 100% unit test coverage. If the code's going to be dropped there's no point in getting 100% test coverage and fixing every corner case.

The waterfall review is based on a much more firmly worded issue. There's little scope for learning about the problem in access database shards in parallel…, so no point in doing the review until the code is complete and all the aspects of the review can be done at the same time.

I wrote above that the names are misleading. What I call waterfall review can be necessary in an agile process. A very agile team where I once worked had the freedom to modify or drop almost every feature as we learned about the problem space. But only almost every feature. There was a contract that demanded that we interoperate with a named third-party service using a specific protocol. Using that protocol was a poor choice, that protocol should be buried, not used, but it wasn't our choice to make.

Most code related to that third-party service had to be reviewed in the waterfall manner, not our usual agile style. I remember a terribly frustrating review where a reviewer wanted to do his usual excellent agile thing and it just wasn't helpful. The contract was signed, the details were fixed and the discussion could not affect anything.

The opposite mistake is more common: Asking waterfall questions even though the team is supposed to work in an agile manner. I think the main reason for that is that the reviewers want to write code, and a waterfall review doesn't require mental immersion in the way an agile review does. Just scan for style aberrations or other surface issues, ask for another code revision and get back to work.


Arak Al-Bustan

It is the last day of August. Tomorrow I start working, bright and early, but tonight I am still on vacation, if unpacking suitcases counts as vacation, and so I have tasted my latest bottle of arak, a variety called Al-Bustam, formerly perhaps made in Oman? There's a place called Al-Bustan there, I bought the bottle in the Muscat airport, and I haven't seen it on sale anywhere else. Formerly is formerly though, today it's made in Jordan, by the same distillery that also makes my favourite arak, Al-Zumot.

I'm glad I bought it.

Al-Bustan is nice, with a round, food-friendly taste. It's not great in the way Al-Massaya is, it just tastes of a happy occasion with good food. Oddly enough I liked it better pure than with water and ice.


Life goes on

This autumn my work shifts from arthouse pixels to mass-market pixels. An occasion to post this remarkable photo:

It reminds me a little of a more modern descendant of the sparse modernist interiors, but sparser, sparser, and with a giant screen instead of art.

The picture comes from an advertisement. You're assumed to like the interior, and you're meant to buy the TV. Too late, that model is not on sale any more.

My work involves avoiding some things you don't see on the photo: A settop box and a hundred-button remote control. Less frustration, more reliability, better usability.


Morbid bindings

The first thing I learned about programming in-person (not from a book or from code) was morbid bindings, a strangely unused term.

I learned it from a university employee called Eric Monteiro, who was oddly clueful in a generally sparsely beclued environment. Can't remember his title, or what he taught.

I had just listened to Eric answer someone else about multiple inheritance, and asked a followup question, which he answered, and then he digressed into relationships in general: When two things are connected in a program, the binding is always one of three: A kind, a belonging, or else it's morbid, and morbid bindings always turn out to be bad in one way or another. He gave me a quick example: This number in this file has to be at the same as that number in that file and I think I answered, such as maximum line length in two functions that read/write the same file? (more…)


Open source is succeeding, and rms is unhappy

This summer Dropbox released an image compression thing called Lepton, effectively a better way to encode JPEG (same principles, same pixel results, considerably better execution of various implementation aspects). Dropbox didn't have to do that. But one does nowadays, it's become part of modern programming culture. Using and releasing open source is a best practice, as the buzzword goes.

Around the same time Richard M. Stallman posted a condemnation of companies that both support free software and teach classes in use of nonfree software. Condemnation is the word he chooses, not my choice. No fraternisation with the enemy!

That enemy is us, now. The enemy is those who follow today's conventional best practices. A stealth-mode startup I am talking to has ten projects on github, because the CTO there has decided that whatever good programmers consider good is what shall be done in his realm. Most of the projects are forks, some with PRs for upstream, others described as the code in this fork isn't really suitable for upstream, but take it if you want. Good, polite behaviour, best practice indeed, and very different from the GNU purity that rms requires.

What is left of rms' following if the good programmers are declared to be enemies? The outlook for the GNU project is poor.


It's the attitude, not the wording

Once upon a time, a clever young programmer submitted a middling patch to the linux kernel. Not directly, it went to a subsystem maintainer who merged it with other code of his own and perhaps from other people, put his name on it and sent it to Linus. The young programmer's name was not visible. That was a bit of luck, because there was a bad bug in the code, the kind of bug that causes blaming, finger-pointing and angry email.

Only Linus, that maintainer and I knew that the young programmer was myself, and Linus never said an unkind word and didn't tell anyone who wrote the code either, so noone could email me and tell me their thoughts.

Fast forward twenty years, to when Sarah Sharp decided to stop contributing to the linux kernel. Sarah wrote good code for the kernel. I was sad to hear that she left, sadder about the reasons.

A lot of people blame Linus and his swearing for her leaving. I think that's unfair to Sarah, unfair to Linus, and worst of all, I think that we in the open source community hurt our community when we do that. Even if Linus' swearing hurt Sarah.

Linus has his faults. For example. when someone from Redhat wanted kdbus into the kernel, Linus' response showed good technical judgement, but it was also heated, loud and negative. Certainly not ideal. However, the fair comparison is not against an ideal. Linus' response ought to be compared to a standard professional response, plausibly something like oh, this code is poor, on the other hand that major stakeholder wants it merged. After a few rounds of squabbling and some cleanup the code is accepted.

That would be professional behaviour. That sort of professionalism is how a good, clueful CTO ends up with 22,000 messy lines of PHP despite a clear mandate from the board to prioritise quality. How teams end up selecting MongoDB over PostgreSQL. Possibly that's how Windows NT got graphics drivers in the kernel.

Linus won't tolerate that sort of professionalism, instead he'll swear and scream. Is that worse? I think not. It may or may not be better, but it's not worse. But because Linus behaves that way and attracts criticism for his loud swearing, some of the less notable opensource contributors escape criticism, and some of them are mean and spiteful as well as loud.

Saying fuck isn't that bad. In my opinion it's about as bad as being late for a meeting. A mean attitude is worse, much worse.

Since people focus on Linus' swearing, though, Linus' swearing opens the door to meanness and spite. A minor fault enables a major.


Animated art

People have started making art using animations on the internet, mostly on tumblr. Great. Most of what I've seen is abstract, some not. All of it is art in a new domain and such a joy. Some examples I like (note that the last one of these probably shows a nude): 1 2 3 4 5 6 7. Several of these are by Florian de Looij, who also makes interesting inanimate art. (Neat word!)


The value of features

A programmer doesn't always know whether a new feature of a program will turn out to be valuable or not. Perhaps: doesn't even often know. I've just had a repeat lesson on that topic.

I have a new phone. The manufacturer brags about a high-resolution camera and many other things, but I bought it because it's the smallest phone with a good screen and an up-to-date version of Android. (Yes it fits in a pocket, even sideways in some pockets.) I noticed in a review on the web that the phone's watertight, complete with an underwater photo of a beauty in bikini, but didn't give any thought to it. After all I don't spend much time in pools or at beaches, and if I do the bikini beauties don't gravitate towards me. When I bought the phone I had no idea that I would care about its being waterproof.

But I do care. The summer rains here in Munich can be impressively intense. Until now I've always been conscious that I was exposing an expensive and fragile electronic device to water when I used a phone in the rain. I have done that when I needed to, but in a corner of my mind I was always aware of the risk. Now I just do whatever I need to do, rain or shine, and don't worry about the device.