Some time ago our project received a new large-screen TV set. With my colleague and friend, László Tarcsányi, we thought it’d be a good means of providing feedback. We also thought it would be great if it was used for showing the current status of our most used project tracks. By status, of course, I mean: build has been ok for a while, broken for a while, currently building etc.
Fortunately, we use Hudson as continuous integration system, which has a very good support for plugins. It didn’t take us long time to find this plugin, called Radiator View (check it out here: http://wiki.hudson-ci.org/display/HUDSON/Radiator+View+Plugin). It can be configured to show one or more tracks at a time, to show/hide stable views, show/hide possible build breakers (people who checked in source code since the last green build). It looks like this:
Broken builds are always shown in the upper part of the screen, in red. In order to be easily noticeable, these red builds cover the most part of the screen.
The idea behind the plugin seems to be working. Broken builds get caught much faster. We had an e-mail based build-status reporting system, but unfortunately that was not efficient. E-mails can be easily missed, the TV on the wall can not (unless you’re walking with your eyes closed). Another problem with the e-mail based solution was that it was sending messages to only that person who broke the build (or to one team, at most). In these situations there might not be enough capacity to fix the build. However, a projected red build is frustrating to all of us. The broken build can be fixed by any person with sveral minutes of spare time.
The TV is also showing the status of our nightly builds. The nice server monitoring application has been created by László Tarcsányi and Kornél Schmeiszer. You can immediately tell how many test cases failed, how many went well and how many test cases were run alltogether (See the screenshot). It looks really nice and can be understood in seconds.
We also project a red-green-refactor picture, in order to keep in mind what’s the right behavior when implementing new functionalities.
Quick and adequate feedback is important. It’s one of the core principles of agile, after all. However, people keep forgetting about its importance and value. They treat feedback related tasks like timewasters; they would even tell at standups “providing feedback is blocking me” if they could.
What is the best and quickest way of providing feedback – audience say with me – yes, code review. When I hear people presenting results of reviews, most of the times, it’s like: “well it’s ok with me…” I guess this effect can be tracked down to two reasons: laziness and a psychological factor.
Laziness is the obvious one. In order to produce quality feedback one needs to be very careful and thorough. Each and every new code line (and not only – I’ll get back to this immediately) needs be parsed, understood and processed. Are variables and methods named in a self explanatory way? Are methods and classes short enough to be understood with ease? Are unit tests in place, are they really testing the functionality, or are they coverage-increaser-liars? Too many questions to be answered in ten minutes. And in most cases people spend just that much time reviewing (usual case, usual developer, the nature of task to be reviewed does not matter). In order to produce the best possible review comments, some systems thinking is also implied. That’s why not only new code has to be processed; a new question arises: how well it fits into the legacy system? Which can be further broken down into a series of “what if” questions. What if we run the code in a multithreaded environment? What if an exception is thrown? What if we refactored things into several classes/methods? Is it possible, that any part of the new code has already been implemented? (that doesn’t start with “what if”, but happens very often in large systems).
The other reason why we don’t do review thoroughly enough, is that we don’t want to hurt authors’s feelings. Many people think like: “well, I could mention this as a possible comment, but what if they take it as an insult?”. Wrong question. In my opinion any suspicious code part has to be mentioned. Of course in a friendly and supportive manner, and not in “what the hell is this bullshit, dude?” style. Some people will take it personally anyway, but they’ll eventually realize it’s for everyone’s sake.
Further reading on producing quality feedback through code review: Zsolt Fabók’s Basics of code review
Well, the following post is about some feedback I’ve recently received on my activity regarding our self-improvement group. I was really about to abandon the whole thing, but my friend Lajos Fülep’s kind words made me continue.
In the last two weeks our little self development group was busy with a brand new topic: namely legacy code (legacy code in the sense of: “code that is working suspiciously, and have no tests around it.”). Some time ago I’ve posted some really ugly code that is full of bugs and has no tests at all. As discussed there, two rounds should be taken: one for adding tests , another one for refactoring it (and fixing the numerous bugs the code contains).
The first round was a pair programming session. The task was to cover the code as much as possible, and spot the mistakes – spot but, don’t touch them (hint: there are at least four major bugs in there). This task can be implemented in an hour. After that, we choose a reference implementation for the second round.
The second round is done randori-based. It means that there is a single laptop along with a projector, while people keep rotating in front of the computer. Everyone is allowed to make a single change. A single change can be: extracting a method, extracting a class, renaming TWO methods, variables etc. Basically the “one change” rule can be adjusted as you wish. I was kind of afraid of this randori-session; I thought people would be afraid of being creative while all fellow group members are looking at them. However, it went really well; people kept saying “I want to stay, this was half a change only”. What is very important here is that anyone not in front of the computer can speak only after the next change to be made has been named by the person at the computer. This means that you cannot give tips what to be done next (only if it was requested explicitly), but can argue whether a change is needed or makes sense at all.
Here’s the feedback I’ve received (translated from Hungarian):
I found this session very useful. What I liked about it was the task itself. It was cool we had to refactor an ugly piece of code. On the other hand it was very useful to me to see how other people were working and thinking. Way to go 🙂
Code refactoring in progress :-).