quarta-feira, 20 de julho de 2016

Code Reviews and the "If it ain't broke, don't fix it" principle

"If it ain't broke, don't fix it." is credited to Bert Lance as the person who popularized it, when he was quoted saying it in the May 1977 issue of the magazine Nation's Business.The expression became widespread, and William Safire wrote that it "has become a source of inspiration to anti-activists." - Wikipedia

Sometimes this principle will be put in place while performing Code Reviews (CR is a verification measure that intends to spot software defects by reading the software source code, as early as possible in the software life cycle). This only means that the main reason you are reviewing the code is to spot defects, so optimizations, formatting issues are out of scope (even if they bother you, don't engage into unnecessary discussions with the author).

Unless, of course, they are part of the written coding conventions for the project (some projects will inherit whatever the company or the customer have). This means that indenting code at 2 spaces instead of 4 could be a defect if there is a coding convention for this team that requires this specific indentation.

Also sometimes, as input to a code review a check list (of different sizes ;) could be handed to you. And those check lists could cover more that spotting defects.

So this principle applies in principle for code reviews unless there is a good reason to break it (Software coding conventions, check lists that are input to each code review).

Got it?