Code Review Boards - How to stay professional

I have recently come across the concept of a code review board. At first it was an awkward moment; My change set was out there for all the world to see and comment. I admit I was stressed about it and with good reason. What I will do in this post is put the record straight, as far as I am concerned, on the good, the bad and the ugly of public/social code reviews. And since in the near future my code will be standing there naked for all to see on some public repository, this should make for a very good exercise on what to expect and what to reject when it comes to review boards. TL;DR Code reviews are good. Review boards meh...

Past (nearly) perfect

At some point during my searching for topics to write I recalled some articles I wrote back in the day on CodeProject. And I also do remember that, unless your article was pure garbage, review of the article and comments where polite, leveled and cool headed. Social media was not that proliferated and people did not openly enjoy putting others in a defensive position on a whim. Oh and by the way one can find links for those articles in this static page in this blog.

The Good

The good thing about having people crowd-review your code is that you get everyone's perspective on what you wrote, be it for the design of your code, potential bugs, or best practices. In a perfect world you can have a professional discussion about it and solve problems, decide on a course of action on technical debts or develop a philosophical discussion.

The Bad

The bad thing about review boards (especially when they are group wide), are the email "noise" they produce. Every single comment will be, most definitely, sent by email to every member in the group at any time of the day... or night.

The Ugly

Although code reviews are good and between professionals are even better, there is always that person with the annoying comments. The person who has something to say no matter what. Who doesn't like your naming scheme. Who thinks that if(A && B)is not clear enough (to this person's opinion) then the elements around the && have to be switched. The petty one. Be polite but precise and assertive to your answers to that person. Be constructive when commenting that person's code. Be professional. Hopefully that person will get off your back. Hopefully but not probably.  

Having said all that, in the link below is the base repository on which quite a few post will be based upon. And it has a bug!!! Can you spot it without running the code (review it)? Can you spot it after reading the code, running but not debugging? Leave a comment below and let me know!

Producer/Consumer Repo 

Comments

Post a Comment

Keep it clean, professional and constructive. Everything else including ethnic or religious references will be removed.

Popular posts from this blog

NUnit Console Runner and NLog extension logging for your tests

Image
Have you experienced the frustration of tests running perfectly well on your local machine and then failing in CI? This all too familiar "It worked on my machine" comes to mind. You set off to investigate and after a while you realize you are too far into the rabbit hole between code and logs in Kibana and this investigation is getting you nowhere. Wouldn't it be nice if you could hijack the logger and print the logs in the test runner's console? TL;DR NLog can be extended easily and NUnit  Console Runner does not play well with simple console logs.   Photo by  Kaboompics .com  from  Pexels Enter Console logging In .Net in general it is not advisable to log to console in a multi-threaded environment, because the console is not thread safe. While logging simple strings may not cause any problems logging strings with interpolated variables may very well be sub optimal to say the least. Should you use any such debugging technique you are better off with a thread safe f
Read more

Tests code coverage in Visual Studio Code with C# and .Net Core

Image
While writing tests for you code goes without saying, there is much to be said about code coverage of tests. How much of your code is actually covered by these tens of tests you just wrote? If you work in a TDD approach most probably you don't have such "mundane" questions. Of course you code is 100% covered. You did make sure every test failed before you added that line in your code to make it pass... and that conditional statement... or did you? While purists would argue there is no chance of no coverage I beg to differ. Let alone if you write your tests after the code is written... After such and such years and experience in the industry when we are nearing deadlines some things are thrown out the window. They shouldn't but they are. TL;DR tools help with coverage. In this case .Net Core Test Explorer with coverlet and Coverage Gutters . Photo by  Startup Stock Photos  from  Pexels Coverlet to the rescue As I blogged a while ago I have made myself a warm
Read more

Applying an internal forums system in the company culture - thoughts

Image
Long gone are the days where, as children, we used to tie a string to two boxes and shout messages at each other. Communication for some of us has changed from phones with a round dial to smartphones in a decade. Having worked in several places with a mobile based solution for groups of teams and dev departments forum/chat system as well as  a couple of places with an internal forum/social media system in place, I have seen more often than not that the adoption rate of such a system is diverse. Some like it, some not and some participate on a whim. And in some places it is obligatory. However what happens when you find yourself accustomed to such a system and arrive at a place where there was none, and one is being applied. How does an organization make the shift from coffee machine pow wows to an internal forum/social/chat board. TL;DR Success of such a forum depends on... oh quite a few factors It is not the medium, it is the content This is not a post on the different solu
Read more