love.huria

love.huria

Recent Posts

Building a code review process that works

Posted by love.huria on Sep 25, 2017 2:57:00 PM

A while back, I talked about the need for maintaining coding standards and how it simplifies the development process for the whole team. The next step is putting in place a code review process.

Why It’s Important?

Code reviews are very crucial for

  • knowledge transfer
  • avoiding small/common mistakes
  • maintaining best practices throughout the dev team

 

Let’s take my current team for example. We are around eleven developers in the team, all producing code which needs to be reviewed. So yeah, that’s a whole lot of code!

Pushing code to production is easy. Anyone can do it, right? What concerns us is the quality of code pieces we are going to deploy. 

The code can be completely fine or it can be a piece which makes everything fall apart. To maintain high code quality, we all need to have peer code reviews. This does not mean that the team writes bad code that needs to be checked. We all are on the same team and we have a common goal, that is to deliver the highest quality product. And a code review process makes sure someone on the team catches the errors that somebody else might have missed.

You must be thinking, “Is it worth it”?  Absolutely yes. 

Not having a code review process integrated into projects can result in big problems. Toyota had to settle a $3 million lawsuit because they did not pay enough attention to code reviews. 

There were a lot of reasons why this incident happened and one of the reasons was an absence of a peer code review. After reviewing their source code they found possible bit flips, task deaths that would disable the fail-safes, memory corruption, single-point failures, inadequate protections against stack overflow and buffer overflow, single-fault containment regions, and more. The list of deficiencies in process and product was lengthy.

Obviously then, it makes business sense to make code reviews a critical part of your development process.

How Do We Do It?

General Development Process

 

General development Process: code review process

This is our general flow which is being followed in most of the projects. Of course, columns may vary depending on the different aspects, clients, and projects.

Assigning responsibility

We have set a few guidelines around who will be responsible for the code review process:

  • There will be one senior and one junior code reviewer for each ticket.
  • Reviews will be done right after the daily standup, depending on how long takes
  • When there are tickets added on the board, one person will review at least one ticket in a day
  • It is both the code reviewer and developer’s responsibility to ensure that all tickets are in respective columns according to the latest update
  • If there is any feedback from the senior code reviewer, it’s the junior code reviewer's responsibility to look what he/she has missed

Github Code review flow

Maintaining your code review process on GitHub is super easy. We can create new projects on our repo and use it as we want. 

Setting projects on GitHub: Code Review Process

Generally, we have four columns, which are:

  • Ready for review: You can add cards (pull requests) to this column if your code is ready to be reviewed
  • In review: Now it’s the code reviewer’s responsibility to move the card to "In review" column so that it gets updated on the branch and the concerned dev understands that his/her ticket is currently being reviewed
  • Change Requested: Again it’s the code reviewer’s responsibility to move the card to this column if the review has failed standards. Then concerned dev will fix the issue and push the ticket back to “Ready for review”
  • Closed/Done: If the card is in the  “Closed” column, that means the PR has passed the requisite coding standards

 Things to look out for

These are the aspects which we consider while reviewing code: 

Making sure the process is followed

We regularly review our way of working against a documented process to identify if there are any gaps in our performance. We also look for ways to improve the process and ensure that it’s not a burden/blocker for anyone’s work.

The process is clearly defined and maintained on different platforms, depending on the client and teams involved. We usually use Confluence and highly recommend it to anyone who is reading this post.

Learning from our mistakes

We maintain a code feedback sheet where we mention common mistakes we need to avoid. Everyone on the team has access to it and can add points on where we can improve, new techniques to achieve certain functionality, coding patterns to avoid etc.

Goals of a Coding Review Process

With a finely tuned code review process in place, development teams can:

  • Enhance the learning of individuals to become better programmers
  • Improving the quality of codebase, even as it grows more complex as we scale
  • Focus on not just quantity, but quality deliverable throughout
  • Maintaining discipline within the team and understand the seriousness of spaghetti code


That’s about it! There would definitely be things I missed here or certain code review practices that are unique to your team. Leave a comment to let us know.

Topics: Architecture, Coding and Tutorial

Keeping it clean: Coding standards that matter

Posted by love.huria on Jun 19, 2017 6:16:00 PM

A year back, I was working on a project that taught me how to keep my code clean, modular, reusable, and all those terms that seem fancy, but are actually good for you in the long run. Interesting? Yeah a bit.
 
But what did I do after getting into those practices?
 
I made mistakes. Believe me, a lot of them. But with every mistake, I learnt a lot of stuff that I had never considered before. It helped me in my thinking process, on how we should build things, what steps we need to consider when we are developing/extending a feature. And most importantly, these learnings were not just personally helpful, but also crucial for team growth.
 
At first we used to get frustrated because we had to follow the additional steps like adding documentation, maintaining changelog files, following the code standards, and keeping them consistent throughout the team. These extra steps seemed cumbersome and we were not able to relate how this can be helpful for the team. And we are still learning/improving everyday in this respect. But after few months we started loving and improvising the process. 
 
So here I am, sharing what I have learnt. And trust me when I say this, once you start doing these, you can't code without following these practices.

This post is focused on what practices we follow everyday to make our lives easier. Although the practices mentioned here are more relatable for PHP/Drupal, they can be easily followed by all developers.
 
Let’s start off with simple things:

Commenting and Documentation Standards

Commenting doesn’t mean adding a bunch of comments and random documentation anywhere while coding. There are things which you should mention to make your colleagues’ lives easier, and yours as well. 

  • Start with writing a description of your component, why you are creating it, and what is the aim you would like to accomplish here, what does it do etc.
  • If there are modifications being done, then those should be logged by creating a Changelog.md file attached to your component. Maintain a specific format to have consistency throughout. 

This is something we follow at Srijan, hope this is pretty clear:
CHANGELOG.md file sample: 

new1

  • It’s good practice to add @see, referencing the class, which will help easily navigate to that class definition by using IDE like PHPStorm, or editors like Sublime etc with just one click.

new 2

 

  • Add @TODOS wherever necessary. It’s very important if you feel your code can be improved in future and you have the ideas on how to do it, but not enough time at the moment. Mention what needs to be improved above that code snippet. One good example could be:

new 3


  • Create README.md files so that others can easily understand what is the working of the module. 

          For example: 

new 4


  • A “Docblock” is a special code comment that offers an explanation of the purpose, arguments, return values, and throw exceptions for a block of code. 

new5

Something very informative in a simple tweet I found:

Formatting

This might include indenting, whitespace, brace positions, and new lines and this can be different according to different languages. In our case, this is specific to PHP (Drupal). There are a lot of plugins available in editors to beautify your code.

Naming Conventions

  • Of course naming conventions depend on the language you are using ( examples: Zend, Google style guides for PHP, CSS, Javascript, Java, etc ) but the main idea is to use descriptive and meaningful words. So, you should avoid calling your variables: xx, yy2, test1, test2 and so on.

  • For example, lower_case for PHP variables and functions, camelCase for JS variables and functions, lower-case-hyphenated for CSS ID's and classes, UPPER_CASE for constants.

  • We should name our variables in a way which easily explains the type of data they contain. Similarly in case of functions, they should describe what kind of functionality they are providing. This is called self-documenting code. Functions should tell what they do, not how. This is called abstraction which allows the underlying implementation to be changed without changing the function name.

Portability

Keep your code as loosely coupled as possible. It is "portable" in that the amount of work required to move it from one platform to another is low. There are few things we should have in mind while coding:

  • Avoid using hardcoded values like absolute file paths, URLs etc  unless it’s a matter of life and death (:P)
  • Avoid using magic numbers in your code. Basically it’s a hard-coded value that may change at a later stage, and hence become hard to update. Almost all numbers other than 0, 1 or 2 should be assigned to a constant at the top of the file. This provides a single point of change if the value changes, rather than a search-and-replace that could affect many files and potentially introduce bugs.

Linters

There are different types of tools available to find syntactic discrepancies in general, especially in interpreted languages like JavaScript, PHP, Python etc. They can also be used as simple debuggers for finding common errors. Here’s a look at the common linters we use at Srijan:

PHP: We use PHP Code Sniffer with Drupal integration. One can easily configure it with your editors such as Sublime which will show common PHP errors on save, which saves a lot of time and prevents errors before committing your changes. 

Javascript: We have

  • We have Drupal JavaScript coding standards (note - these vary in several ways from Mavericks standards) in place but we use JSHint for listing our JS related code checks. 
  • Wherever Drupal's JS formatting conventions conflict with JSHint, JSHint supersedes. 

SCSS:

  • We do not have any SCSS related documentation on Drupal.org but we do have Drupal CSS coding standards (these can be applied to SCSS code).
  • You can find some documentation related to SCSSLint here.
  • Also it’s good to checkout Compass best practices as well. 

Reusability

This is something we are working on quite extensively. We are building reusable components which can be used in across different websites (which have almost the same purpose). These components will provide the basic functionality and styles which can be overridden in different websites according to their own requirements. The most important thing here is to identify what functionality can be turned into a component.The degree to which your code is reusable is largely based on how tightly it’s coupled with the rest of your code base. A good example can be building a banner slider which can be used in most of the websites.

Modular

This basically means keeping your code independent of others, so that one bad change to your code does not break everything else. This is similar to the concept of coupling in object oriented programming. It’s like breaking the website into its basic independent parts of a more manageable size.

Use Continuous Integration Tools

We use Travis CI. It’s a distributed continuous integration service used to build and test projects on github. The service is free for open source projects. You  might be wondering why you didn’t you use it before! ? Don’t worry, It’s never too late and pretty easy to setup with your github repositories.

  • First step is to register Travis-CI, which you can also do with your github account.
  • Setup the .travis.yml file, This file handles the build of your environment and also the execution of the phpunit files.

You can check the simplest basic configuration here:

new6

When you have the phpunit test in place and if it is passed, it will show something like this on your commit:

new7


Simple Travis setup: https://github.com/lhuria94/drupal/pull/3

  • This .travis.yml should be at the root of the project.
  • Travis only runs builds on the commits you push after you’ve enabled the repository in Travis CI.

Code Reviews

We have pretty awesome code review process in place. But this blog is already too long, so I shall cover that in my next blog. Stay tuned. 

Meanwhile, you can check out our webinar by Elijah Manor, on how to sniff out JavaScript code errors.

Topics: Drupal, Coding and Tutorial

Discussion

Write to us

This site is protected by reCAPTCHA and the Google Privacy Policy and Terms Of Service apply. By submitting this form, you agree to our Privacy Policy.

See how our uniquely collaborative work style, can help you redesign your business.

Contact us