Meet the Critic: Code Inspection System in Opera Software

The internal source code inspection system Critic, used in Opera Software, was posted on Github under the Apache License 2.0 last night.

Sometimes code inspection systems are scolded for not being completely adapted to the commercial development process. This is not about Critic. Critic was tested in the process of commercial development of software in large projects of a large company, and it showed itself perfectly. I highly recommend trying this wonderful tool to you.

Download the Critic source codes here: github.com/jensl/critic .

Critic was written by Opera, a Swedish developer named Jens Lindström. He was not satisfied with the existing code inspection systems, so he decided to write his own. By the way, Jens is famous not only for this. Those who are interested can search his posts in the internet about the JS engine of the Opera Carakan.
')
Critic was written for the specific needs and tasks of the Opera. The author of this tool was also its user. He tried to make the tool practical and useful both for himself and for other developers. As a Critic user who watched his growing up and fouling with useful features, I think Jens succeeded. Critic turned out to be a convenient and efficient tool that saves time for its users.

Critic is a Web application and is written in Python. Critic is tightly integrated with Git. Your commits are added to review as soon as you push them into the Git repository, which is monitored by Critic.

The inspection cycle takes place as follows:

1. The author review commits its code in the Git-repository, which is monitored by Critic.
   
2. Depending on the commit method, review can be created automatically with git push, or manually via the Critic web interface.
   
3. As soon as the review is created, the inspectors and observers of the code that has been changed will be notified by e-mail.
   
4. Inspectors and observers can add problem notes and notes to the code. Only inspectors can mark a modified code as inspected. Inspected code does not mean approved. This means that the inspector checked it and created a record of all the problems that he found.
   
5. The review author participates in the discussion of problems and notes, and also pushes new commits into review that fix the problems found. New commits are flagged as non-inspected. Items 4-5 can be repeated many times.
   
6. When all commits are inspected and there are no open problem records left - the review is considered approved.
   
7. An approved review may be rediscovered, as well as, for example, a bug in BTS. Closed problem records may also be rediscovered. And of course, you can add new problems to the code you have already inspected. Philosophy is like in BTS. There is a specific workflow, and it is usually followed, but if needed, you can return to the previous phase of this workflow.
   

Some notes:

1. Problem entries may apply to the entire review, commit, or specific lines of code. These lines of code may not be part of a commit, i.e. may be part of an “intact” code, for example, when a commit changes the behavior of an intact code. If a problem record refers to specific lines of code and these lines are corrected in the next commit, the problem is marked as “addressed” by this commit. The new commit, however, is marked as un-inspected, so the entire review cannot automatically become approved. This is one of the features that saves time when working with the system.
   
2. Critic supports history rewriting. It sounds scary, but it is useful, especially when “rewriting” occurs on a parallel Git branch via git rebase -i. For example, if your review has grown to 50 commits, and many of these commits — little things like minor bug fixes, compilation fixes, variable renames, comment edits, and other “cleanups” - will litter your VCS history. In such cases, it is useful to “stick” all these intermediate fixes to the commits that they fix before giving up their branch for integration into the main project code. Thus, in the main branch of the project there will always be a beautiful story, not a mess. Well, almost always. So, if you performed such an operation and turned 50 “bad” commits into 5 “good” ones, with the same content, Critic will not ask you to inspect this code a second time. For this to work, the total diff of 50 bad commits must be 100% identical to the total diff of 5 good commits. The rewritten history will be published in review.
   
3. Critic also supports branch point changes. Suppose you branch the review branch from the top of the main project branch. During the inspection, the main branch of the project went ahead. And your branch needs to change the code in accordance with the new realities of the main branch. What to do, create a new review and lose all the work already done on this review? As if not so. Critic will help you here too. All commits can be copied to a new branch point, using the same git rebase, and tell Critic exactly where you moved the branch point. And he will understand everything! And she would not ask to inspect the same code a second time. And do not ask to inspect tons of code from the main branch. But he will notice that you have resolved the conflicts in question, and will ask the inspectors to check whether you did it correctly. Well, isn't it great?
   
4. Critic supports extensions. “It's fashionable now.” Extensions can obviously extend the functionality of Critic and save you even more time. Extensions can implement various specific functionalities that not all users need. Therefore, each user installs himself the extensions that he needs. Suppose you are working on a project H. Fixit bugs. In review, initially there was 1 commit for bugfix, but the inspector got very strict, and I had to commit another 5 fixes to fix to satisfy them. Finally, the review is approved. And you have to do a bit of simple, but tedious, formal work on commit commit. Someone from your project decided to make life easier for his teammates and developed an extension for your project. The extension adds the button “Integrate bugfix into project X” to the Web-interface. Now you can press this magic button, and Critic will do a lot of useful things for you:
   
   • Collect your 6 commits in 1.
   • Will offer him a comment from the first commit.
   • Will suggest to edit the comment.
   • Check if there are merge conflicts with the main branch of the project that has gone ahead, or the branch for bugfixes.
   • Zakommitit fix to the branch for bugfixes.
   • Close review in Critic.
   • Will write something good in BTS.
   • Closes the bug in BTS.
   
   
5. Critic is not a wrapper around the main project repository, such as Gerrit. Therefore, Critic can be used for both pre-commit review and post-commit review as convenient for your organization. Critic, however, can pull up the code from the main repository, or share one repository with it. And also push the code there using extensions, as shown in the previous paragraph.
   

I hope Critic will be useful to you. Good luck in your development!