Review (2)


Sonar Comments on BitBucket Pull-Requests

At work we have a scramble to use static code analyzers to improve the quality of code in general. Both from a security perspective and from a standardization perspective. I have worked with Sonar before, but it has almost always been in the background, alone and forgotten by everyone who are pushing features. Now those who know me are aware that i prefer early feedback, preferably pre-merge. I like to think of the Patch, Pull or Merge request as the real guard against flinchy developers like myself who don’t have time to run the tests, or check sonar for issues that should be fixed while i’m covering that particular code. This article is about resolving that and getting sonar comments directly on pull-requests.

Requirement

  • TeamCity as a build server
  • C# classic as software platform
  • MSBuild as a build system
  • BitBucket cloud for a source repository .

High level design

This is what it looks like from a high level. A Pull-Request in BitBucket triggers a TeamCity job that, in turn, runs the same pull-request builder build-process as would be done with a regular pre-merge job but with a sonar-analysis in preview-mode and a specific sonar-plugin that is able to post comments.

Prerequisites

Things you should probably do before delving in to all the configuration.

BitBucket

  • A specific user that can be named Sonar-Reviewer and added to your team

TeamCity

Make sure you build the pull-request trigger from master branch if the latest release is still pullrequest-20172603195632 since it needs the fix in this PullRequest by yours truly to be able to post the pull-request id to sonar. mvn package with maven should create the zip you need)

SonarQube

Configuration

There aren’t that many things to setup for this to work actually.

Configuration in BitBucket

Configuration in Sonar

  • If analysis is protected then create a system user for TeamCity to login to sonar

Configure TeamCity

  • Set the JAVA_HOME variable to where your JRE 8 is for each agent
  • Make sure any proxies the agent should use to post to api.bitbucket.org is also specified in the SONAR_SCANNER_OPTS environment variable, either as agent property or as build parameter. In my case i had to se env.SONAR_SCANNER_OPTS=-Dhttp.proxyHost=myproxy.tld -Dhttp.proxyPort=1234 in the AGENT_HOME/conf/buildAgent.properties.
  • Configure a pull-request trigger to look like this
  • Make sure your VCS root has the following branch specification: +:refs/heads/*
  • Go to parameters

    • Add the following parameters, it’s of course possible to skip the ones you don’t want configurable
    • Make sure you added the OAuth Client Key and Client Secret from your BitBucket user created earlier
  • Go to build steps

    • Add Sonar Analysis Begin step
    • Set a project key, version and branch as you see fit, they may not be empty but they are not important for this either
    • Add Sonar Analysis begin with the following huge parameter list with the following Additional CommandLine Args

/d:sonar.analysis.mode=preview /d:sonar.bitbucket.repoSlug=YOUR_REPOSITORY /d:sonar.bitbucket.accountName=YOUR_ORGANIZATION_OR_USER /d:sonar.bitbucket.oauthClientKey=%sonar.bitbucket.oauthClientKey% /d:sonar.bitbucket.oauthClientSecret=%sonar.bitbucket.oauthClientSecret% /d:sonar.bitbucket.pullRequestId=%trigger.pullRequestId% /d:sonar.bitbucket.minSeverity=%sonar.bitbucket.minSeverity% /d:sonar.bitbucket.approvalFeatureEnabled=%sonar.bitbucket.approvalFeatureEnabled% /d:sonar.bitbucket.maxSeverityApprovalLevel=%sonar.bitbucket.maxSeverityApprovalLevel% /d:sonar.bitbucket.buildStatusEnabled=%sonar.bitbucket.buildStatusEnabled%

Make sure it corresponds to the parameters you added before. Save the build step.

  • Add a MSBuild step with whatever targets you want. Sonar for MSBuild suggests MSBuild.exe /t:Rebuild
  • Add a Sonar Analysis End step with default settings

That’s it!

At this point you should be able to create a pull-request, see the job trigger in TeamCity and have the sonar-plugin work its magic and post any issues introduced by the PR as comments like this.

I’m especially happy i was able to put this integration in place, seeing as i had no prior C#, Sonar Analysis for MSBuild or TeamCity experience experience. But it all get’s easier with time, and most integrations look similar and require the same kind of tinkering.




On Code Review

I’m a professional, i will code as i wish I like code-review, it’s generally a great idea to have it incorporated early whenever code is being written to create business value. Mostly because it is cheaper to fix bugs early abd before they break something in production and starts sending nagging users your way. By having a few extra critical eyes on your code you can stop many of those embarrassements. But it not only finds bugs, it also spreads knowledge within the team, increases the bus-factor, and gradually increases the competence of the developers in that team. This has been talked and written about lots and lots of times before, so don’t take my word for it. Many companies, SmartBear and atlassian for example, has their own motivational articles about why it should be used, so don’t take my word for it. Even coding-horror has written a piece about it. The effect of good code-review is not unheard of!

So why another piece on Code Review?

But i still encounter people who do not see the point! Code review is sometimes seen as a cumbersome process that only steals time and does not have any return on investment, essentially slowing down feature development. Hold your horses, slowing down? Yes, slowing down feature development, because of ${BOGUS_REASON}. I heard many of them, if it is not this then that. It is being made reasonably clear that the attitude towards code review is being clouded by team stress, management, the idea that manual labour is okay this once.

The Great List of Bullshit I think we should change every poor attitude we meet about it. So i have collected some of the arguments i have heard against code-review.

I’m the only senior developer so nobody has any real input for me First of all, the only reason you are a senior developer is because educators, lecturers, professors and developers senior to you have taught and coached you into becoming the grand developers you are today. So

it’s time to pass the baton, use code-review as a means to get junior on par with your level of expertise oh chosen one. Secondly, everyone can have some input on any code, you are not an omnipotent deity. Perhaps they will come in with a perspective that is more modern, or unfamiliar to yours.* Learn to appreciate feedback*, the worst thing that can happen is that your code improves.

I know what i’m doing, i don’t need code-review, that’s for junior devs Then show off your great examples on how it should be done, by performing letting people review your code and understand your greatness.

My code doesn’t have bugs, i have a great workflow That’s simply just not true, nobody writes flawless code all the time. But if you do, great, why aren’t you famous? Because most of us aren’t awesome elite developers and the sooner we realize that the sooner we can use a workflow that allows feedback and constructive criticism on our work.

It is expensive, we shouldn’t spend time on it First of all, do you realize how expensive it is to have untrained junior developers writing code without supervision? Have a look at

raxos502’s Terraria clone and find out just how horrible code can become when in the hands of someone new with software development.

It takes time to build and verify that the code actually fixed the thing/built the feature Use proper Continuous Integration on patches going into your code and review that only when static code analysis, unit and integration tests have gone green. It should never be up to the reviewer to acceptance test the code, that is what testers and product owners are for.

It doesn’t replace testing Of course it doesn’t, testing is another beast to tame and code-review doesn’t replace it or even strive to solve the problems that testing solves. If someone in the business tries to claim that it does then they shouldn’t be allowed to make decisions about it.

I don’t like it You don’t have to. But code-review is about reading code, which is what you do all day. So if you don’t like it then why are you in this industry?

Why code review is awesome

  • School new developers to know about the quirks of a project. And let’s face it, every project has them.
  • Coach, code review is likely the best way to coach junior developers into their new role. To invest in them early is probably the best way to get them to deliver value to the team.
  • Spread the knowledge, by having multiple people code-review your code you make more and more people aware of how the project grows and in what direction it expands.
  • Enable remote work, when you have no idea on the remote developers skillset or when you are working with offshore or open source projects.

How to make it so

Informality is quintessential when it comes to code-review, anyone should be able to do it and give their seal of approval. Gerrit has something that’s called +1 and +2, and it is awful in the sense that you enforce lead/senior and junior developers and gives a select few the ability to approve all the code. In my experience this has never worked out well. Continuous Integration and a workflow resembling Git* Flow. Connect as much as you can to the code-review platform, which should be close to the repository. I personally like how GitHub, Gerrit, GitLab and BitBucket does it. They make the pull request or patch trigger web-hooks that in turn trigger Jenkins or some other CI Server. With that connection in place the possibilities are endless. Take some time and let the team get the full power of CI in addition to the code-review. Break the culture of single omnipotent developers, because it never lasts because they eventually leave and when they do you can be sure that the product they had their poisonous hand over is dead in the water.