Skip to Content

We often hear that code reviews are frustrating and waste of time. Really?? Or is the lack of adoption of a suitable, well-defined process that is the root cause of it appearing futile. Think again!!

There are many articles highlighting the importance of doing code reviews, listing out to-do and not-to-do instructions, as well as explaining different code review alternatives (automated, peer review etc.), hence I am not detailing those here.

Through this blog, I want to stimulate the thought and highlight the importance on “how” the “right” code review process tailored per your “organization structure and need” play a significant role in embracing the code review mind-set within the project team and stakeholders.

By “right” code review process, I mean it is accepted and easily integrated within your software development/release life cycle (SDLC). It shouldn’t look disparate, additional step or hindrance.

To evolve to “right” and robust code review process, your first step is to ensure that the code reviews are encouraged and it happens. This is truly possible only if there is awareness on its importance and have buy-in from key stakeholders. It is not only the developers that drive it but sincere acceptance and encouragement from Project Managers, Business leads, Analysts and End-users that primarily contribute to its success.

I am part of SAP Development team and feel proud to say that in my current organization this process is thoughtfully customized, neatly defined and well-integrated with other phases of SDLC.

We have clearly written code review check-list, coding standards document that is easily accessible to stakeholders. These documents contain answers to FAQs, security related coding norms, tips/pointers to improve performance and also inputs to write code for easy maintainability and support globalization/reuse.

To effectively implement the code review process, we have a dedicated code review team which is an independent, unanimously recognized group that governs the process and the coding standards. It functions across modules and projects and owns accountability of code changes moving to production environment.

The code review process is also tightly integrated with the subsequent Change Control process (process that focuses on moving code changes to production) in SDLC. The change control mechanism checks for code review status and warns when a code that is not reviewed is proposed to move to Production. In such case, it alerts and triggers action points for stakeholders to take appropriate action.

I am highlighting key points that have worked well for us

Mind-set:

To develop this mind-set, project team is kept informed and educated via different forums(seminars, blogs, trainings and question hours) to have their buy-in and feedback. This has helped in evolving to “Right” process.

The Team:

Forming of “dedicated”, “independent”, “unbiased” group is key to its unanimous acceptance. The role and responsibility of the team is clearly defined and accepted. The team has a good mix of experienced professionals having wide-ranging technical understanding.

Documentation:

The content is simple, precise and easy to understand without missing out on the exceptions/specifics. It is easily available to all. Any updates to it and its communication are well governed.

Deep integration:

The process is thoughtfully tailored per our need and is well-integrated with other phases of Software Development Life Cycle (SDLC). Though an independent process, it is an integral part of SDLC.

With the above thoughts on the code review process. I open up for further discussion and invite you to share your experiences on it within your organization.

To report this post you need to login first.

9 Comments

You must be Logged on to comment or reply to a post.

  1. Lucas Tétreault

    I think that having a dedicated, independent and unbiased group doing your code reviews is a mistake. The whole development team should be responsible for code reviews. Maybe you limit it so that only senior developers are able to approve review requests… Either way, it should be people who are familiar with the code and have a vested interest in keeping it clean.

    When we first committed as a team to doing code reviews we just promised we would do them and had no tools or technical barriers which forced us to do it. Now we use Phabricator (Code Review with Phabricator – an open source, software engineering platform ) and can’t release transport tasks without an approved request. There is an option to override the review process but that triggers an email to the entire development team to let them know.

    (0) 
    1. Amol Vakhare Post author

      Hi Lucas,

      Thanks for sharing info on Phabricator. I will surely go through your blog and related articles.

      Coming back to your comments on dedicated, independent and unbiased group, I would like to clarify that the code reviewers are very much part of our global development team. It is from a role clarity point of view that we refer them as a Code Review Team. They have special privileges and are authorize to approve/reject code changes. Needless to say, that the code written by a code review team member is also reviewed by some other member of the code review team. No custom code is exception. Hope I am clear here.

      (0) 
      1. Matthew Billingham

        Even so, I favour peer code review over an independent team.This means that not only is the code review work shared across the entire team, but experienced developers are kept on their toes and junior developers learn from the experienced. Once a junior has been reviewed a few times and got the hang of the process, there’s no reason for them not start reviewing other’s code.

        On projects I’ve been on with this practice (which is many, going all the way back to 1997!), it has worked rather well. There comes an element of healthy competition. I remember one chap I was paired with, and we would go through each others code with a fine-touthed comb, trying to catch each other out!

        In some industries, like the Pharmaceutical industry, code review is a requirement of the regulatory authorities such as the FDA.

        (0) 
    2. Joao Sousa

      People who supposedly have a vested interest in clean code also have an even greater interest in getting things done fast. From experience, trusting developers to keep to standard on their own doesn’t work.

      I believe quality review is done better by people from outside the project. Not knowing the context is better as the reviewer has less prejudice, and will ask the questions no one asked before. An outside perspective is critical.

      (0) 
      1. Lucas Tétreault

        Trusting developers to keep to standard on their own doesn’t work.

        I kind of feel sorry for you if that is the type of people you work with. While trusting one person may not be ideal, a team of people with an agreed upon goal of quality, maintainability and adherence to standards will always produce better code.

        I don’t know why you think that I meant someone on the same ‘project’ should review the code… First, my workplace uses Scrum so we don’t have ‘projects’. Second, our reviews are performed by seniors working in any area of our application and no constrained by ‘projects’, ‘products’ or any grouping of functionality.

        I totally and 100% disagree that an outsiders perspective is critical. A true outsider won’t understand the intricacies of the code and won’t understand the design patterns being used. An outsider won’t be able to give any valuable feedback and has no reason to invest and real time or thought in to it since they don’t have to look at and maintain that code.

        (0) 
        1. Joao Sousa

          Quality is a touchy subject in all areas not just development. As far as development is concerned, we probably have diferent experiences because you seem to work on larger development probjects while I work with ABAP on an ECC implemenation/enhancement context where I have smaller teams of 1 or 2 programmers.

          Unfortunately I’ve seen too many dinosaurs in my day.

          Regarding the outsider that person has a.specific QA role so the reason is doing his job. Speaking from my perspective even if I’m outside the project if in the future there is a problem people may ask for my help, I will lose time and budget fixing it. One time (fortunately only once in more then 10 years) I had to fix the damage done by an unreleased function module and I was an outsider at the time of the development.

          QA should be done by management level people, with a vested interest in the company at large.

          (0) 
  2. Jelena Perfiljeva

    Not sure why it didn’t get more likes or better ratings – this is a good subject and I felt sharing in a blog “what worked for us” was very appropriate and helpful. Well, maybe many SCN members do indeed believe that code review is “waste of time” (which is quite scary!).

    I can’t stress enough the importance of having good standard / guidelines in place and having a democratic review process for them. Many of us have probably seen the documents that are full of ABAP “urban legends” or simply stuff that some “senior developer” believes even though it’s not supported by any evidence.

    In the last review of our own document I found the DSAG guidelines to be very helpful.

    (0) 
    1. Joao Sousa

      Democratic….well…. 🙂

      What I consider critical is that standards are as objective as possible and all people agree that the standard is not “the right way” but rather the opinion of the people that designed them. You may disagree, you may even be right, but that’s not the point.

      Having gone through some painful experiences, I’m sure code review is critical.

      (0) 
  3. Shibha Jain

    Along with having an independent code review team and good standards / guidelines in place, the other two points mentioned in the blog, mindset and deep integration into SDLC process, are also equally important. What I have seen in my experience is that these are missing links mostly,

    Mostly the project managers and analysts don’t really understand the importance of Code review and hence they dont have the patience to wait for code review before moving changes to Production, For them it is just a mere formality. it is not even considered while estimation and building project schedule. 

    And neither is this process one of the mandatory steps of software development cycle so it becomes easier to move changes ahead without the proper reviews.

    (0) 

Leave a Reply