The last entry in my Going Agile series will look at the retrospective meeting.

The retrospective meeting is held at the end of every sprint/iteration, and it is the time to discuss what went well, and what could be improved in the next sprints. Some people will say the Product Owner should be in attendance, and some believe the PO should not. IMHO, the PO is a part of the team, and should be there…and in our case, I was. We weren’t sure how to solicit input from the team, so we decided that everyone should take a few minutes to write down their thoughts, and then the Scrum Master would read them out. This was a good way to eliminate the classic, “I was just going to say the same thing as Bob” response. After all the responses were collected, we realized we had 3 main things to address:

1)      Testing and documentation struggled…they were too heavily back-loaded

2)      Code reviews were determined to be essential but weren’t being factored into estimates, etc.

3)      Our team velocity was nowhere near what we thought it would be

From these things we made a few adjustments for our next sprint (again, these decisions were made by the team as a whole). Our developers stopped working on new stories 2 days before the end of the sprint, and would then focus on testing and documentation. This would help alleviate the avalanche of new functionality that would hit the testing and documentation team on Friday afternoons. Code reviews were added to the definition of ‘Done’ and were factored into to estimates.  For the 3^rd issue,  we found one of the key issues to be that developers just weren’t given dedicated time to code, and as such, could be interrupted at any time for an impromptu meeting, or discussion, etc. We decided to implement a Do Not Disturb mode for the developers, and if they had that DND sign up in their cubicle, or on IM, then they were not to be disturbed.

The retrospective is a crucial part of the continuous improvement process, and time must be dedicated to it. The first few are extremely important since that is when the warts are most obvious, but minor tweeking  may never stop.

I’ve enjoyed sharing my experiences about my first Scrum team, and I hope it may provide some ideas for your team. If you have any Agile/Scrum experiences you would like to share, I’d love to hear about them. Chances are others will stumble across the same problems at some point as well.

Part II – Joy is the word…

OK, so Grease is really the word, but it didn’t fit my theme, gimme a break… Anyway, back on topic, since Joy of code review – part one of this series was published last year we’ve seen our new code review product in action in a variety of customer and prospect situations, and much like the eponymous hair product in the musical mentioned above, what we thought of as an interesting twist on an existing paradigm has turned into a bit of a barn burner. I refer, in this case, to the notion of what constitutes a code review if you remove the formalism of the invite from the process.

Consider what I’ll call, for the sake of being what marketers insist on terming “edgy” (for no really good reason as far as I can make out), old fashioned code reviews. You know the type, we talk about how we really should do more of them all the time. Check in your code, mail out a bunch of invites, mail some more when those get declined, gather around a table, project your code and wait for the insults to come rolling in.

You want to try that again, Mr. Coding Specialist...?

On the down side of these things are all the obvious problems… People don’t like getting reviewed, and unless you have a particularly unpleasant architect, the reviewer is no happier about being in the room than the person on the sharp end. Factor in the time, the annoyance of the arrangements, the opportunity cost of yanking the architect away from whatever they were previously doing, and you’ve got a really expensive, not very productive, but very important from a pointy-haired-manager-perspective process.

It’s really the classical no-win situation. Your manager requires it to be done. You hate it, and you know everybody else in the room hates it too. It’s like a giant dose of spinach to a five year old – doesn’t matter how good it is for you, you’d rather scream and sit in the naughty chair all day than let that stuff past your lips.

So when we were thinking about changing the approach to code review, it seemed obvious to us that whilst code review itself is valuable, the means by which it gets accomplished is fundamentally broken. Factor in peoples’ unthinking delight when confronted with anything social and what the heck, we figured, let’s turn the whole thing on its head. Instead of going top-down into a software organization and helping the manager enforce something unpleasant in an all new and collaborative-y, enterprise-y way, how about reaching out and encouraging bottom-up engagement through a model that people are comfortable with anyway, namely formless (a.k.a. social) communities.

Who’s the most obvious person to review the code of a good developer, after all? It might be their architect, but the chances of a good developer making a blunder of the architectural type (or any kind of dumb error) is probably reasonably low. Not saying it doesn’t happen, but we pay people at that level a good amount of money on the understanding that they produce decent code, so why then treat them like kids? Instead, if the code produced by that guy is made available for anybody to review, quite literally, then rather than getting the architect grumpy because he’d rather be thinking about the next huge money maker than what this guy happened to have done mostly right but nit-pickingly-wrong in this one situation, you get other team members taking part who have (in most cases) more useful input to impart anyway.

Instead of feedback of the “so… rather than using that particular transitive constructor, I’ve found that explicitly instantiating a new object and then initializing only what I need saves me, on average, 3 cycles a day” type, you might get the “hey, I was hacking on that a while back… might want to filter that data, cuz Bob’s front end passes in all kinds of crap… just saying” type instead – your choice, but personally I’d rather hear an hour’s worth of the latter than a moment’s worth of the former…

So who is at the review turns out to be much more important than whether it’s held, given some arbitrary set of “holding” conditions. But of course this comes with its own set of challenges, notably how do you know when you’re done if there’s no formal “meeting” to review your code (and to insult you, have we mentioned that part?).

In fact, it’s much like how the transition from waterfall to Agile was accompanied by many a gnashing of management gums and misplaced wails of “but how will I know if it’s going to be done on time?” But hey, that didn’t work out so bad, did it? People got used to time boxing, to changing requirement sets, to not waiting until it was arbitrarily “finished” and instead shipping it so as to gather feedback faster.

In my next post I’ll look at this new world order from the top down and examine the benefits to encouraging (rather than imposing) a social code review paradigm, and how it can make those metrics we know you care about look better than ever before.

Part I – Ode to Joy

Since the launch of the seminal “Joy” work which hopefully doesn’t need mention here, we’ve seen everything from The Joy of Cooking to The Joy of Not Working (my personal favorite!), and so further to that deeply mined vein of authoritative works we bring you the necessarily over burdened… Joy of Code Review!

Joy, you say? Let me count the ways…

• I implement a task, using what I consider to be best practice patterns and guidelines; I slave over this, my creation, and when it’s done, I stand back and admire, much in the tone of an old master, this latest image of my greatness.
• Then I remember I need to get it reviewed…
• So, I timidly invite my Architect and 3 of his best friends to the war room to review my new baby
• After many rescheduling pauses, we finally gather…
• I hold my breath, turn on the projector, and bare my soul to the collective seniors in attendance
• 30 minutes later, having endured a ritual mind flaying, and the predictable but nevertheless enjoyable tortured examination of my parentage, education, upbringing and such fun rhetorical musings as “why do they let people like you graduate?” I slink out
• Follow up is, if anything, more painful as I’m reminded moment-by-moment of just how badly I’ve lived up to the expectations laid out for me by the senior team members

Anyway, so code reviews suck, amirite? But we all know we need to do them. Of course, we all know we need to do them for completely different reasons from each other…

• Kids right out of grad school know they need to do code reviews because although their code is, like, totally perfect, it’ll be good to show the old dudes their skillz, and for the old dudes to check out some rad new stuff that they might have missed along the way.
• Senior guys know they need to do code review because otherwise all kinds of terrible cruft will get promoted into the head branch and somebody (are you looking at me??) will have to fix it…
• Managers know they need to do code reviews because they read all about them in a book with a cool cover, and it’s all Agile and stuff, and let’s face it they’re being measured on code review coverage, so come hell or high water you’re going to do code reviews!
• And of course, regular professional developers know that code reviews, however painful, genuinely lead to better code, regardless of the pain involved in getting there.

What we have here, folks, is a social organization, complete with the crazy uncle, the embarrassing grandma and the pimply teenagers. And social organizations, as we’ve all come to know and love, are at their best when the forum in which they’re fostered exists for a reason that encourages the unstated, but nevertheless in-your-face activity of which those in the respective societal groups are desperately in need:

• Facebook? Getting a date. And then getting another one while simultaneously trying desperately to avoid the previous partner. Rinse/repeat. Seriously, I have no idea how kids manage today. At least when I was young and awkward we could hide behind the silence and foot shuffling of real face-to-face meetings. Now with a keyboard and the internet in the way, there’s nowhere to hide!!!! I’m off topic again… ahem…
• Linked-In? Getting a job.
• Myspace? Getting a clue.

You get the idea.

So code review as a social engagement… really? Parts 2 and 3 of this series of posts will examine how such interactions, fostered by social networking tools, are the best way to ensure code review gets done and returns value both to the participants and to the companies in which they work.

In part one of Going Agile,I talked about how we introduced Agile to our development team. This next post will look at the events that led to our first iteration planning meeting.

During the weeks that led up to Iteration 1, there was much work that went on as a team, and much that each team member did individually. As the Product Owner, my biggest task was to create a backlog. Sure, I knew what the main new features were going to be, but I still needed to capture this, and add other oft-requested features. I scoured every correspondence I had with customers, sales, support, development, and so on to gather this information.

After everything was said and done, I had a pretty massive backlog… a pretty massive, unprioritized backlog. At this point, I really didn’t know any good techniques for backlog prioritization (that would change after attending the CPO training with Mountain Goat Software). This training was not going to happen for a few months, but something needed to be done… so I did what any good Product Manager does…I used the ‘wet finger in the air’ technique. Now, my ‘estimations’ were based on a number of concrete data points and some not-so-concrete assumptions and anecdotal evidence, so they weren’t totally of the ‘wild-assed guess’ ilk. After a few more days I had my backlog read for the team.

While the backlog creation was going on, a number of team meetings were occurring. Two of the more important meetings involved creating rules for the team and preparing our definition of “Done” . I highly recommend spending some time up front on both of these activities.

Creating the team rules was a great exercise, because it was the first time the team sat down as a collective and decided what the rules would be. Many of the rules were not groundbreaking…things such as everyone’s opinion is equal, treat everyone with respect, don’t be late for meetings, and when and where daily Scrums were going to happen.

The best result from this meeting centered on the team’s communication methods. Everyone was already using email, so that was covered. Instant messaging was rolled out to the team, and everyone was to use it. Of course face-to-face discussions were encouraged the most, but there needed to be some way to let people know you didn’t want to be disturbed (unless something was urgent). Everyone created a Do Not Disturb sign, and when it was posted, it was to be respected. Sometimes people just need to focus on the task at hand, rather than constantly being disrupted. We came out of that meeting with a clear set of easy-to-understand team rules, and we posted these rules in our team conference room for all to see. Note… rules can and will change over time.

Next was coming up with our definition of Done. The team sat down for a couple of hours to determine what should/should not be included. Looking back, we thought we were cavaliers and were blazing new trails with the definition we came up with…in reality, we put together a definition that was pretty much in line with the ‘industry norm’. One thing that we did not include initially was code reviews…that is, for a story to be considered done, the code had to be reviewed by at least one other developer (who was not associated with that piece of code). During our Iteration 1 retrospective, we modified our definition of Done, and code reviews became part of it. In fact, this definition of Done may go through many, ahem, iterations before becoming finalized.

Finally, we needed to get our ‘room’ set up and have all the necessary supplies on hand. Our team decided to use a wall board with color-coded cards for the tasks. Green cards were for development tasks, red cards were bugs, blue cards were for testing tasks, and yellow cards were for documentation tasks. Now we just needed a board to pin these tasks to. We didn’t want to spend a small fortune on a big pin-board, so we got creative and used carpet under padding. (You can get a huge piece of this at any DIY store for next to nothing and it works like a charm.) We fastened it to the wall, put on some masking tape borders and labels, and we had ourselves a Scrum board.

So with our prioritized backlog, team rules, definition of Done, and Scrum room all set, we were now armed and dangerous, and ready for our first iteration planning meeting…TO BE CONTINUED.

In response to Jason’s post about false positives.  First of all there is a general misconception of false positives.  Modern static source code analysis tools have changed the game.  It is not the Lint tool of the past, a focus with deep inter-procedural technology has placed the requirement that static tools today produce more real issues than false reports.

With that said, Jason is right, large code bases never running static analysis will produce a large number of issues no matter how accurate it is.  Even though static analysis tools do provide a number of ways to manage this (and Jason talks about one) it does make sense to put this in your code reviews. You are looking at legacy code but if you are doing code reviews then you must have changed something with that legacy code.  Therefore having those bugs visible to you during the code review could suddenly now apply.

Jason certainly hits the nail on the head.  Automation, specifically using static analysis, is key and it should be tightly integrated with your code review. Although we need to be careful where we label source code analysis.  Static source code analysis certainly can find those low level issues such as labeling your local variables correctly, but it goes beyond simple code style issues.

Where static source code analysis can really help is with the deep inter-procedural context that it can provide.  For example, during a code review you go through some code with a number of function calls.  Hopefully you know what each and every function is doing…but do you really?  This is where the deep analysis of static source code tools can help.  It can help you identify that there may be an issue in the code review and that issue happens to show that a function is returning NULL.  Uh oh, potential null pointer dereference on our hands.

Now add code reviews with other static source code technology, such as full source cross reference information, flowcharting, impact analysis for any function/methods and architectural representation to show you the full context of the system.  Now you’re talking powerful.