Let Your Process Go
I would still love to hear your feedback in the comments below. Enjoy!
In my previous work place, we were using ClearCase for source control. We were programming in a Unix environment and we we’re using cleartool for our ClearCase needs. We didn’t use a workflow control tool or anything. Code reviews were mandatory, but it wasn’t enforced by a higher power. The team was self-governed in this respect. Code reviews were conducted 99% of the time. It was pretty fun.
In my new workplace, we also use ClearCase. But with ClearQuest, and on Windows. There’s a similiar process here: you write your code, someone gives you a code review, you check your code in. But here, this process is “automated” with the ClearCase + ClearQuest beast. When you check out a piece of code, you have to have a ClearQuest Activity associated with it. Your team manager approves it and assigns a reviewer. When you’re done with the code, the reviewer inspects it, gives you the comments, you do a little back and forth and when you’re done, your team manager closes the activity and you can deliver (a ClearQuest jargon for “merging to your parent’s branch”) and all is well.
I hate this.
But why? It seems like it’s pretty much in-line with the same process I feel is important. Code reviews are in there with unit tests on my “must do” list of how to build software. Let me explain. One of the first things I did while being trained for this new job is compiling some code. Someone from my group, let’s call him Kevin, helped me with it. It turned out that there was a bug in one of the Makefiles in our code base that someone else, let’s call him Joe, was supposed to fix. It was just a small bug - there was a line break in the file and someone accidently deleted the “" character for line breaks. We popped down to Joe’s office and asked him if he fixed it yet. He told us that he hasn’t gone around to it. We went back to my computer and Kevin did something strange - he changed the Makefile locally on my machine without checking it out (it’s called hijacking in ClearCase). Why was it strange to me? Because he fixed the bug on my machine. Why not check it in and get it over with? I asked him this very question and Kevin replied “Joe already has an Activity open to fix this. In any case, it’s too much of a hassle to get into right now”.
Shock and awe.
Developers are too hassled to change stuff they see in our code base. I overheard two people talking when one of them said “Come on, I don’t want to create a new Activity for this, just push it into one of your features’ Activity”. I get them. The process of creating an Activity, have someone assigned to be a reviewer, all this time wasted on correcting some docstring? I wouldn’t be hassled either.
This is where our “automated” workflow solution breaks. Sometimes, SOMETIMES, it’s okay to break the process. Does restoring a line break character really necessitates a code review? How about fixing a typo in a method’s documentation? Does it really needs an administrator’s approval?
This brings me back to one of my favorite quotes, from Alan Kay (I even use it on my online cv):
Simple things should be simple, complex things should be possible.
– Alan Kay
Fixing typos should be simple. If you were working alongside someone (a-la pair programming), a further review might be redundant. Updating documentation shouldn’t neccessarily require a manager’s approval. If you trust your developers with coding your product, you should trust them not to abuse your process.
Discuss this post at the comment section below.Follow me on Twitter and Facebook