Semi Automatic Code Review

Talks

Richard Huang

#continuous-integration-ci

#development-process

Semi Automatic Code Review

by Richard Huang

In this talk titled "Semi Automatic Code Review," Richard Huang presents a framework for improving productivity in code reviews, particularly in Ruby on Rails projects, as part of the development life cycle. This framework addresses the challenges that arise when code quality is not prioritized in the early stages of a fast-growing company.

Key points discussed include:
- Development Life Cycle: Richard outlines the typical development process which includes new feature implementation, bug fixing, manual code reviews, and QA testing prior to deployment.
- Code Review Importance: Emphasizing the critical nature of code reviews, he notes that they can identify bugs and performance issues, albeit being a manual process that can introduce complexities.
- Automation Opportunities: Huang suggests that while many stages of development can be automated, code reviews still largely rely on human evaluation. He introduces the idea of partially automating this by utilizing tools that help analyze code based on best practices.
- Rails Best Practice Gem: Richard details the creation and functionality of the Rails Best Practice gem, which analyzes Rails project code and offers suggestions for refactoring. He highlights the benefits such as better readability and adherence to coding guidelines. For example, it can suggest using scope access for querying posts belonging to the current user, and using built-in methods to simplify code.
- Realbp.com: In response to the needs identified during code reviews, Richard discusses the launch of realbp.com, an online service that integrates with GitHub. This service allows developers to track code quality over time, share analysis results with teammates, and customize what checks to apply based on team preferences.
- Plugins and Customization: Richard explains the extensibility of the gem through plugins, allowing teams to adapt the checks to their specific coding guidelines.
- Practical Demonstration: He provides a step-by-step guide on registering a GitHub repository with realbp.com, analyzing code, responding to suggestions, and tracking improvements.

In conclusion, Huang stresses the dual focus of semi-automatic code reviews: improving code quality while enabling engineers to dedicate more time to critical performance and scalability concerns. The main takeaway is the push towards leveraging automation in the code review process to ultimately foster better coding practices and enhanced collaboration among engineers.

00:00:25.320 Hello everyone, let's get started. My name is Richard, and I come from China. Today, I will be discussing the topic of semi-automatic code review, a process that can significantly improve your productivity during code reviews. I am currently working for G, maintaining the open platform, which is the largest mobile social gaming platform. Additionally, I am active in the open source community; I created the Rails Best Practices gem and recently developed the rb.com website. You can check out my GitHub account to find more useful tools.

00:01:17.200 This is our development lifecycle for each sprint. We implement new features or fix bugs, then ask other engineers to perform code reviews. Our QA team verifies the changes before we merge them into a release branch, and finally, we deploy everything to the production server. Most teams using Rails have a similar process. In this development lifecycle, some processes can be automated while others must be performed manually. Manual work can often introduce bugs into the product, whereas automation makes tasks simpler, more accurate, and less prone to errors.

00:02:01.000 In our development work, it's evident that we manually write code, but sometimes we generate code automatically. We also write extensive unit tests and integration tests to ensure code quality. However, code reviews are generally done manually. Sometimes, this involves sitting face-to-face and reviewing code together; other times, we send a code comparison link and leave comments on GitHub. In our QA process, we utilize a continuous integration test server to ensure all unit tests and integration tests pass before deployment. We also write a lot of scripts to automate regression testing, but manual verification is still necessary during the merge process.

00:02:35.360 We use Git, our version control system, to automate the merging of feature branches into the release branch. Conflicts are resolved manually if they occur. Regarding our deployment process, we are thankful for tools like Capistrano, which allow us to release new features almost entirely automatically. As you can see, we have varying levels of automation in every process throughout the development lifecycle.

00:03:10.160 However, the code review process stands out because it is crucial for identifying most bugs and potential performance issues. It heavily relies on the engineers involved. For instance, if engineers are well-versed in Ruby and Rails, they can suggest opportunities to utilize many of Rails' default helpers and methods. If engineers have a strong understanding of databases and HTTP protocols, they may recommend optimizations for database queries and suggest adding a caching layer.

00:03:42.800 So, we asked ourselves if it would be possible to partially automate the code review process. The good news is that it can be done, and this is what I want to discuss today. Let's delve into what code reviews entail in general. They contain two parts. The first part is straightforward: its goal is to make code more readable and maintainable. For instance, you should adhere to your team's coding guidelines, write better coding syntax, and remove unused methods.

00:04:07.960 The second part is more complex but equally important, as it requires reviewing whether the code is performant and scalable. This aspect largely depends on programming experience. What if we could automate the first part of the review process? This would allow engineers to focus their time and attention on the more critical aspects of coding.

00:04:27.440 Over the last two years, I have built the Rails Best Practices gem, which analyzes the source code of Rails projects and provides suggestions for refactoring and improving code quality. For example, consider the edit action in top code. This code retrieves a post from the database and checks user privileges by simply verifying if the current user is indeed the post's owner. The Rails Best Practices gem will inform you that there is an issue in the post controller. It suggests that instead of directly accessing the post, you should use scope access to fetch posts associated with the current user.

00:05:00.000 In this way, if the post does not belong to the current user, a not found error will be raised. Similarly, consider the top code example where, before creating a post, the user ID is assigned from the current user. After analyzing this with the Rails Best Practices gem, you will receive a suggestion to use model associations, just like in the previous bottom code. It will recommend creating the post using the current user's posts, which automatically sets the user ID.

00:05:29.440 Another example includes suggesting the use of Rails' default predicate methods. In the top code, the code reads the user's login attribute and checks if it is present. The Rails Best Practices gem will advise you to use the query attribute instead. You can directly call 'login?' on the user object, simplifying the check for attribute existence. I've noticed more and more developers beginning to use the Rails Best Practices gem. Some integrate it into Garden, running the gem frequently in development mode, while others have incorporated it into Jenkins to run it on their continuous integration servers.

00:05:58.120 I have also seen teams building internal tools based on the Rails Best Practices gem to score their developers' code. While I appreciate such initiatives, I prefer a more collaborative approach. In our team, we decided to address technical debt. We have an extensive Rails codebase that we started in early 2009, and it contains many unused classes, methods, and outdated code. I used the Rails Best Practices gem to clean up and refactor our codebase.

00:06:34.560 However, the Rails Best Practices gem generates its analysis in the terminal, which isn't the easiest way to share results with the team. I wanted to easily communicate with authors of the code I aimed to refactor, discussing whether code could be removed smoothly. This prompted the idea of building an online code review service, leading to the creation of rb.com early this year. The platform integrates with GitHub, allowing the online code checker to execute automatically every time you push code to GitHub.

00:07:07.760 Moreover, sharing analysis results with your team's collaborators is straightforward because it is all online. The Rails Best Practices service keeps track of every analysis result, enabling you to monitor whether your code quality is improving. You can even configure what to review. For example, if you are not concerned about the use of query attributes, you can disable this checker on rb.com.

00:07:46.920 Here's how it works: First, register your GitHub repository on rb.com. Then, every time you push code to GitHub, GitHub will notify rb.com about the new push. Subsequently, the Rails Best Practices gem checks out your source code and analyzes your code quality, generating an analysis report that you and your collaborators can view.

00:08:06.760 This is how the analysis report will look. It displays the commit ID, commit message, and lists all the code issues identified in the repository. For each issue, it shows the file name, line number, warning message, and which commit introduced the issue. Highlighted lines indicate that the code issue occurred in the current commit, allowing you to quickly identify problems.

00:08:42.920 Moreover, by clicking on a warning message, you can access a page that provides suggestions on how to refactor your code. Clicking on the file name will take you to the GitHub file page where you can directly modify your code. This system enables you to conduct preliminary code reviews on your own, improving code quality and saving time for other engineers.

00:09:17.440 The history tracking page displays a visual chart representing how many code issues have been recorded in your repository over time. A table lists every analysis result, allowing you to click on any item to go to its specific analysis report page. I understand that not all checkers in the gem suit every Rails team's needs. Some Rails teams may adhere to different code guidelines.

00:09:47.840 But don't be discouraged; the Rails Best Practices gem allows teams to extend its functionality by creating custom plugins. For example, I want to find instances in my codebase that could use the Rails 'try' method, which returns nil for non-existent attributes. I wrote a plugin for this purpose to replace manual attempts with the `try` method.

00:10:14.440 The tests for this 'try' checker plugin are readable and straightforward. The first two tests check if I have used `nil?` instead of the `try` method for attributes. The Rails Best Practices gem alerts me of any outdated patterns in my code. Developing plugins is not particularly difficult, and such additions can help ensure consistent code quality within a team's codebase.

00:10:53.440 You can share these plugins publicly by publishing them on GitHub, making it easy for others to benefit from your contributions. As a demonstration, I created a simple Rails project on GitHub, integrating it with Rails BP to show how it works. I will now register the repository on rb.com by entering the GitHub repository name, which will synchronize the information.

00:11:58.680 Once the repository is set up, you'll be assigned a token. Make sure to copy this token and update the respective setting on GitHub. This ensures that every time you push code, the Rails Best Practices service will notify us for analysis. After pushing the code, you will be directed to the analysis report page where any code issues will be visible.

00:12:31.080 On the analysis report page, you may see several code issues. If one of the warnings suggests replacing an instance variable with a local variable but it is not relevant to your project's context, you have options to adjust checker settings. Lets, for instance, uncheck a particular checker if desired, update the configuration, and push the code to see the changes.

00:13:12.840 When accessing the report again after pushing code adjustments, you will notice that the previously outstanding code issue for instance variables has disappeared. If any issues remain, such as a suggestion to use scope access, clicking the provided message will link you to a detailed explanation means you can easily refactor your code accordingly.

00:13:55.600 Another example could involve adding methods to your model class. For instance, I might add a method called `admin?`, which always returns false. Once pushed, the analysis report would indicate that the unused method exists, as it was added but never called. To address this, I can modify the controller to implement logical conditions such that only an admin can perform a particular action, allowing the integration of the `admin?` method into the overall logic of the application.

00:14:53.760 Once this change is done and the code is pushed, the report should reflect that the issue with the unused method has been resolved. As I clean up my codebase, I always pay attention to finding and removing such unused methods that are rarely called. This approach promotes a cleaner, more maintainable code environment. With that, I will close my presentation. Thank you for your attention.

RailsConf 2012