A Review of a Review

We worked a lot on our DueDates projects this past week, and we decided it was time for another code review.

I reviewed the Green Team's implementation of the DueDates project, but honestly, there wasn't much to review. The Green Team didn't create a branch of their code for me and Erin to review; they also didn't create a review issue to indicate what areas of their project we should specifically address. I suppose it wouldn't have made much of a difference anyway, though, because their system wasn't much different from the DueDatesBootstrap system that we all started out with, so there really wasn't much to review about it. I commented on general things like building and running their program; their classpath variables and importing their project into Eclipse; running CheckStyle, PMD, FindBugs, JUnit and Emma; and their documentation.

Our (Erin and my) DueDates project was reviewed by John Ancheta, Daniel Arakaki, and Mari-Lee Flestado of Team Purple. The comments tended to focus on the required review criteria, such as their experience building and running the program, and whether or not our code passed various tests. Some of the reviewers had trouble setting the classpath and environment variables correctly and, as a result, had trouble with our system. It is hard to say whether these reviewers were just confused, whether our documentation is inadequate, or whether there was an actual problem with our system. Some of the comments were less than helpful. For example, one reviewer suggested that we create unit tests to check some private methods, but I am not sure how to write tests for private methods. Some comments were even wrong (e.g., suggesting that we make an interface method private, which is illegal for interfaces). One comment that I did find helpful was that someone pointed out that there was an error when running the Javadoc Ant target. I had been running Javadoc from Eclipse and had forgotten about this Ant target, so I would not have caught this error.

Overall, I was dissatisfied with this code review. I did not learn anything from reviewing the Green Team's project because they did not work very much on it. I also did not learn much from reading our code reviews. I think the problem was not so much the reviewers as it was the review process and specifically, the required review criteria, and the time constraint. The reviews focused too much on the required criteria which, to me, are relatively minor details that I could easily have checked myself. I was hoping for a more in-depth examination of our code--what we were doing poorly, what we could improve, what we didn't think of--so that I could learn to be a better programmer. Perhaps the problem was this review process, or perhaps I was expecting too much out of this code review.