The Upsource Blog : Code Review and Project Analytics | The JetBrains Blog https://blog.jetbrains.com Developer Tools for Professionals and Teams Tue, 09 Aug 2022 16:18:02 +0000 en-US hourly 1 https://blog.jetbrains.com/wp-content/uploads/2023/02/cropped-icon-512-32x32.png The Upsource Blog : Code Review and Project Analytics | The JetBrains Blog https://blog.jetbrains.com 32 32 Sunsetting Upsource https://blog.jetbrains.com/upsource/2022/01/31/upsource-end-of-sales-announcement/ https://blog.jetbrains.com/upsource/2022/01/31/upsource-end-of-sales-announcement/#respond Mon, 31 Jan 2022 13:59:57 +0000 https://blog.jetbrains.com/wp-content/uploads/2022/01/DSGN-13152-sunset-banners-for-Upsource-end-of-sales-announcement_Blog_Featured_image_1280x600.png https://blog.jetbrains.com/?post_type=upsource&p=219655 As of February 1, 2022, we will no longer sell new licenses or renewals for Upsource or Upsource user packs. We will continue to provide technical support and critical security updates until January 31, 2023. After this, no further updates or technical support will be provided.

Why we are discontinuing Upsource

Over the past decade, we’ve seen a gradual industry trend of moving away from standalone code review tools to those integrated with other areas of software development. 

In order to address this need, we tried to offer integration of Upsource with our other teamware tools. However, we realized that not only was this limiting what we could achieve, especially in terms of a pleasant user experience, but it also created additional installation and maintenance burden for end users. As such, we decided to focus our efforts around code review on JetBrains Space, our integrated solution for software projects built from the ground up. 

If you are interested in learning more about Space and how it compares with the functionality offered in Upsource, please read the detailed comparison document.

Your options as an Upsource customer

First of all, as an existing Upsource customer, you hold a perpetual license which allows you to use Upsource indefinitely. In addition, you can upgrade to the latest version covered by your current subscription. 

At the same time, we understand that even if you’re happy with Upsource, you may have certain requirements to only use fully supported software. For these cases, we are offering a migration option to Space. 

If you purchased your Upsource license on or after January 1, 2020, you qualify for a special offer for Space. You can receive a discount equal to double your annual spendings for Upsource since 2020. This offer can be applied to any of the Space plans available*. 

We are extremely grateful for your support over the years and hope to continue working with you moving forward. If you have any further questions, please do not hesitate to contact Upsource support or the sales team

*Note: the on-premises version of Space is currently in the Early Access Program, and we plan to release it around Q2 2022. If you’re interested in joining the EAP, please contact our support team. The special offer will also be applied to the on-premises version once it has been released.

]]>
https://blog.jetbrains.com/upsource/2022/01/31/upsource-end-of-sales-announcement/feed/ 0 https://blog.jetbrains.com/zh-hans/upsource/2022/01/31/upsource-end-of-sales-announcement/ https://blog.jetbrains.com/tr/upsource/2022/01/31/upsource-end-of-sales-announcement/ https://blog.jetbrains.com/ko/upsource/2022/01/31/upsource-end-of-sales-announcement/ https://blog.jetbrains.com/fr/upsource/2022/01/31/upsource-end-of-sales-announcement/
What’s New in Upsource 2020.1 https://blog.jetbrains.com/upsource/2020/06/25/whats-new-in-upsource-2020-1/ https://blog.jetbrains.com/upsource/2020/06/25/whats-new-in-upsource-2020-1/#comments Thu, 25 Jun 2020 11:16:16 +0000 https://blog.jetbrains.com/upsource/?p=1740 This release introduces a variety of improvements related to GitHub and GitLab synchronization as well as some other features. Please read the release highlights for details.

DSGN-9381 Upsource 2020.1 release_1600x800_ Blog

Merge reviews for GitHub and GitLab integration

Upsource 2020.1 comes with the ability to create merge reviews for pull requests from GitHub if synchronization with GitHub is turned on. The merge review diff resembles the GitHub pull request diff and lets you easily compare your branch with the target.

Merge_reviews_for_GitHub

Pull request synchronization improvements for GitHub

We’ve made a few other improvements related to synchronization with GitHub.

  • When the review is completed in Upsource, it’s now shown as approved in GitHub.
  • When you add reviewers in GitHub, they are automatically added in Upsource.

reviewer

GitHub webhooks

In a synchronized GitHub-based project, Upsource periodically polls the GitHub repository not only for new commits but also for changes in pull requests and comments. Now to get updates without any delay, you can set up a webhook on the GitHub side, which will notify Upsource of any changes the moment they occur.

Comment synchronization with GitLab

Now when you leave comments on a diff or a source file in GitLab, these comments are also reflected in Upsource. This feature also works the other way too, so Upsource comments are reflected in GitLab.

gitlab

Code review workflow improvements

Now after reviewing and accepting the changes in a code review, you can merge and delete the branch right from the review page.

workflow

Other improvements

Quickly navigate to reviews from the search field. Collapse/expand all changed files in a revision. We’ve also added a new webhook type for adding and removing the review label, ability to set up multiple issue trackers for an Upsource project and ability to manually mark files in a review as viewed.

Please check our issue tracker for the complete list of changes.

To try out Upsource 2020.1, download the build – but don’t forget to backup your current instance!

Download Now

We highly appreciate all your feedback, so please share it with us in the comment section below.

]]>
https://blog.jetbrains.com/upsource/2020/06/25/whats-new-in-upsource-2020-1/feed/ 1
Customer Story: Upsource in Feedonomics https://blog.jetbrains.com/upsource/2020/02/28/case-study-upsource-in-feedonomics/ https://blog.jetbrains.com/upsource/2020/02/28/case-study-upsource-in-feedonomics/#comments Fri, 28 Feb 2020 08:43:46 +0000 https://blog.jetbrains.com/wp-content/uploads/2020/02/upsource-Upsource-Feedonomics1.png https://blog.jetbrains.com/upsource/?p=1730 Upsource & Feedonomics1

Hi everyone!

Today we are happy to share with you how Upsource is used by Feedonomics, a company that combines best-in-class technology and service to list products everywhere people shop online, including Google Shopping, Amazon, and Facebook. Feedonomics services many of the world’s most prolific advertising agencies and brands, including over 30% of the top 1,000 internet retailers.

We were happy to have a Q&A session with Brian Roizen, a Chief Architect and a Cofounder of Feedonomics.

Could you tell us a little bit about yourself?

Hi! My name is Brian Roizen and I am the Chief Architect and Cofounder of Feedonomics. In addition to overseeing all of Feedonomics’ Automation Processes, I also love taking the most annoying manual tasks and automating them. I am excited to discuss how Upsource has helped us up to our game when it comes to code reviews across our engineering team.

What does your company do?

Feedonomics is a technology platform that allows eCommerce stores to easily optimize their product data and list products on hundreds of different advertising channels like Amazon, Google Shopping, and Facebook.

When did you start using Upsource?

We started experimenting with Upsource in May 2019, with a full adoption across our entire engineering team in July of 2019.

Which features do you find useful for your company’s processes?

We love the inline commenting feature, isolated commit reviews, and branch tracking features. These are of course standard features, but they are really implemented extremely well and are easy and simple to use. CodeIntelligence is fantastic as well because it’s great for enforcing proper coding styles and static analysis of our PHP code. Before that, it was a very manual process and, as such, we would miss small issues.

What’s your approach to the code review process?

Code reviews require at least one owner of the software to review and approve the change. We try to keep changes small, when possible, but that is often easier said than done. For large changes, a big part is making sure nothing will break current behavior, while new features can later be validated further on production and tweaked if necessary.

How do you measure the effectiveness of code reviews?

How long the review takes to complete is important to measure for efficiency. As for effectiveness, ultimately it is tied to the effectiveness of the product. How many releases are made that need to be patched or reverted later?

Since you introduced a code review in your company, do you think the quality of code has changed?

Absolutely, the quality has improved even while we’ve doubled the size of our engineering team in the last year.

What did you use before, and why did you choose Upsource?

Before, we were using git diffs directly in our terminal. Visibility wasn’t quite as nice, of course, but the real issue was not being able to use inline comments – you get so much more context when you are viewing a comment right inline with the code – plus we like the ability to track their resolution.

Do you use any other JetBrains products?

PHPStorm, and we love it!


We’d like to thank Brian and Feedonomics for taking part in this Q&A.

If you want to share your experience with us, please let us know by posting a comment or by contacting us.

]]>
https://blog.jetbrains.com/upsource/2020/02/28/case-study-upsource-in-feedonomics/feed/ 4
Upsource 2019.1.1460 is available! https://blog.jetbrains.com/upsource/2019/07/24/upsource-2019-1-1460-is-available/ https://blog.jetbrains.com/upsource/2019/07/24/upsource-2019-1-1460-is-available/#comments Wed, 24 Jul 2019 12:33:40 +0000 https://blog.jetbrains.com/upsource/?p=1720 Upsource 2019.1.1460 is out!

Upsource 2019.1 was failing to start on some Linux systems. In this update, we have fixed this critical issue and made some other improvements. We recommend everyone to update to this latest build.

Check out the release notes and download the new build.
If you have any questions, please post them in the comments below.

]]>
https://blog.jetbrains.com/upsource/2019/07/24/upsource-2019-1-1460-is-available/feed/ 1
What’s New in Upsource 2019.1 https://blog.jetbrains.com/upsource/2019/06/27/whats-new-in-upsource-2019-1-2/ https://blog.jetbrains.com/upsource/2019/06/27/whats-new-in-upsource-2019-1-2/#respond Thu, 27 Jun 2019 13:58:38 +0000 https://blog.jetbrains.com/upsource/?p=1706 CnsBLnca

Great news everyone – Upsource 2019.1 is out! We have added a number of features and improved the performance to smooth out your code review process. It also comes bundled with IntelliJ IDEA 2019.1 and Hub 2018.4, and supports servers running Mercurial 4.9 and newer.

For more details, read the release highlights.

A new type of review: merge reviews

In Upsource 2019.1 we have added a whole new type of review. Merge reviews let you check the changes that will be merged into the selected branch and find any conflicts before merging.

Merge_review

Manage review labels automatically

A new custom workflow takes care of adding and removing the review labels when reviews are created and revisions are added to a review.

Manage_review_labels

Review description

You can now provide additional information about the changes and review objectives in the dedicated review description section.

review_description

Ability to archive projects

When a project is archived, Upsource stops polling the VCS repository for changes and disables the creation of reviews. By marking legacy projects as archived, you can communicate their status clearly to the people who view them and improve the performance of large installations.

Archive_project

Mention groups in discussions

Sometimes you need to get everyone’s feedback on a code discussion, so we have added the ability to bring it to the attention of the whole group.

mention

Specify review deadline using working days

Weekends are now taken into account when setting a review deadline.

Review_deadline

Webhooks for reactions

We have made webhooks available for the addition and removal of comment reactions.

Webhooks

You can see the complete list of changes in our issue tracker.

To try out Upsource 2019.1, download the build from our website, but don’t forget to backup your current instance!

]]>
https://blog.jetbrains.com/upsource/2019/06/27/whats-new-in-upsource-2019-1-2/feed/ 0
Important Security Notice – Vulnerability allowing permission escalation https://blog.jetbrains.com/upsource/2019/01/18/important-security-notice-vulnerability-allowing-permission-escalation/ https://blog.jetbrains.com/upsource/2019/01/18/important-security-notice-vulnerability-allowing-permission-escalation/#respond Fri, 18 Jan 2019 14:44:54 +0000 https://blog.jetbrains.com/upsource/?p=1699 Please note that if you a commercial customer of Upsource, you should have already received an email from us in the middle of December. No further action is required if have already seen this email.

What happened

During a regular security audit on December 7th, 2018, we discovered a security vulnerability in JetBrains Hub, which provides authorization and authentication services to some of our other products including Upsource and YouTrack. This security vulnerability affected Upsource instances starting from version 2018.2.1013 through version 2018.2.1141 where the issue was fixed.

What information was compromised

This security issue affected all Hub instances and other products that rely on Hub, making it possible for users to elevate the permissions that were available to their own accounts in Upsource and YouTrack.

We don’t have any information to confirm whether access to your Upsource or YouTrack installation was compromised.

What actions we’ve taken

We fixed the issue on December 10th, 2018 and released updated versions of Upsource on December 18th, 2018. We’ve also added automated tests to check for this vulnerability whenever changes are deployed to the code base.

What actions you should take

Please upgrade to the latest build from our website.

While it is possible for you to determine whether your data was compromised, due to the nature of the vulnerability, disclosing how this would be done could affect other Upsource installations.

If you need any further assistance, please contact our Support Engineers.

]]>
https://blog.jetbrains.com/upsource/2019/01/18/important-security-notice-vulnerability-allowing-permission-escalation/feed/ 0
Upsource 2018.2.1154 is available https://blog.jetbrains.com/upsource/2019/01/02/upsource-2018-2-1154-is-available/ https://blog.jetbrains.com/upsource/2019/01/02/upsource-2018-2-1154-is-available/#respond Wed, 02 Jan 2019 11:23:20 +0000 https://blog.jetbrains.com/upsource/?p=1697 Upsource 2018.2.1154 is out!

We have fixed some regressions introduced in 2018.2. The bundled Hub was updated to version 2018.4.11067.

Please note that this update is recommended for all users. Check out the release notes and download the new build.

]]>
https://blog.jetbrains.com/upsource/2019/01/02/upsource-2018-2-1154-is-available/feed/ 0
Upsource 2018.2.1141 is available https://blog.jetbrains.com/upsource/2018/12/18/upsource-2018-2-1141-is-available/ https://blog.jetbrains.com/upsource/2018/12/18/upsource-2018-2-1141-is-available/#respond Tue, 18 Dec 2018 14:38:58 +0000 https://blog.jetbrains.com/upsource/?p=1680 Upsource 2018.2.1141 is out!

We have fixed some regressions introduced in 2018.2. The bundled Hub was updated to version 2018.4.11052.

If you’ve already upgraded to 2018.2 from an earlier version, please do the following after installing this update:

  1. Run `lib/upsource-console.sh` from the Upsource home directory (or `lib\upsource-console.bat` if you’re running Windows).
  2. Run `fix-deleted-reviews` from the console.

Please note that this update is recommended for all users. Check out the release notes and download the new build.

]]>
https://blog.jetbrains.com/upsource/2018/12/18/upsource-2018-2-1141-is-available/feed/ 0
What’s New in Upsource 2018.2 https://blog.jetbrains.com/upsource/2018/12/05/whats-new-in-upsource-2018-2/ https://blog.jetbrains.com/upsource/2018/12/05/whats-new-in-upsource-2018-2/#comments Wed, 05 Dec 2018 13:22:00 +0000 https://blog.jetbrains.com/upsource/?p=1652 Please welcome Upsource 2018.2!

800x500_blogUP_2018_2_

The latest version of Upsource brings you a number of nice improvements to simplify the process of creating and updating reviews. There’s more automation and a lot of it is enhanced, so you can focus on coding and reviewing code instead of performing admin tasks. There’s also extra support for Java 9, and, of course the usual updates to the IntelliJ IDEA engine that manages the code warnings and suggestions.

More Automation

Creating and maintaining code reviews should not be a painful part of your development workflow. Upsource already has many ways to automate the creation of reviews, and 2018.2 introduces the ability to automatically create a review from commit messages that match some predefined pattern, or prevent certain commits from creating an automatic review.

commit-messages

And, of course, you can combine this with the other triggers. For example, you could have Upsource automatically create a review for any commit that has a message containing a specific topic and assign to the expert in that area.

You might think that one of the downsides of such a streamlined process for creating reviews is that it would not be able to take into account other factors such as a reviewer’s availability. But with Upsource 2018.2 it’s now possible to mark yourself as out of the office. This means that reviews will not automatically be assigned to you, and also give your team visibility of your status so they don’t have to figure out whether you’re around to review code or not.

out-of-office

It’s possible to set automatic deadlines for reviews. The deadline is calculated as a number of days after the review is created, which can be configured. This deadline will automatically be moved if the review is updated, for example with new code or new reviewers.

Simpler Review Process

You can now see summary information about a review with a tooltip on the commits page. This gives you key information about the review without having to click through to another page.

Upsource 2018.2 lets you remove multiple revisions from a review, so you can easily update which revisions make up the review in a single step.

remove-multiple-revisions

Java 9 and above

Good news if you’ve made the jump up from Java 8. Upsource 2018.2 supports syntax highlighting, code intelligence, and navigation for Java 9, 10, and 11. If you’re using Java 9, don’t forget to check out our What To Look For In Java 9 Code blog post.

java11

And Finally

You’ll notice the Upsource UI has been updated to give you a more consistent experience. This is not only nicer looking, but should also make it easier to use. The performance of Upsource has been improved, and support has been added for servers running Mercurial 4.6 and newer, all of which should give you a better user experience.

Upsource 2018.2 is here to make it even easier to perform code reviews.

Download Now

We highly appreciate all your feedback, so please share it with us in the comment section below.

]]>
https://blog.jetbrains.com/upsource/2018/12/05/whats-new-in-upsource-2018-2/feed/ 2
Webinar recording: “Code Review Best Practices” https://blog.jetbrains.com/upsource/2018/10/16/webinar-recording-code-review-best-practices/ https://blog.jetbrains.com/upsource/2018/10/16/webinar-recording-code-review-best-practices/#respond Tue, 16 Oct 2018 16:28:34 +0000 https://blog.jetbrains.com/upsource/?p=1630 The recording of our webinar “Code Review Best Practices” is now available on JetBrains YouTube channel.

In the past we’ve looked at the specifics of what to look for in code review and code review best practices in Upsource, in this webinar recording we take a step back to look at what we’re trying to achieve with our code reviews, so we can maximize the value of the code review and minimize the pain.

https://youtu.be/a9_0UUUNt-Y

About presenter:

Trisha_Gee
Trisha has developed Java applications for a range of industries, including finance, manufacturing, software and non-profit, for companies of all sizes. She has expertise in Java high performance systems, is passionate about enabling developer productivity, and dabbles with Open Source development. Trisha is a Java Champion and is a Developer Advocate for Upsource and IntelliJ IDEA.

Q&A

As promised, here are Trisha’s responses to questions that have been left unanswered during the webinar.

Q: How do I get developers to see code reviews as useful?

If you’re already doing reviews, and the developers don’t think they’re helpful, then you need to understand what value of the review you’re after and make sure the team understands it too. You may need to tweak the review process so it delivers that value. There’s also the possibility that if you’re doing reviews and the developers don’t see them as useful, they actually might not be useful – take a step back and figure out what’s going wrong.

If you’re not already doing reviews, the best way to persuade developers that code reviews are a good thing is to make sure they get something out of it. For example, you could focus on the learning aspect. Make the objective of the reviews “to share knowledge and learn from each other”. This way the reviewers get something out of the code (they learn stuff) and the authors get something out of it (they taught something to someone). Once code reviews are part of the process, and developers are getting value from it, you should start to see improvements in quality/readability. At this point if you need to you can tweak the processes goals.

Q: Is it helpful to have output from sonar in the code review tool? Or does it just add noise?

Yes, I think it’s useful for output from automated tools to be visible as part of the review process. For example, Upsource shows the results of the TeamCity build – this way, a reviewer doesn’t waste time reviewing code that doesn’t compile or has failing tests.

Similarly for static analysis tools – it can save a reviewing from checking these things manually, or if some of the results are a matter of opinion, the reviewer can make a comment on whether it’s valuable to fix the issue or if it’s not that important.

Q: I hear conflicting views on annotation, why is it some say that annotations should be none to limited?

I believe you should annotate your code as an author in the context of code review comments, not code comments, e.g. comments in Upsource not comments in Java. You can explain which aspects you considered when you picked a particular design/approach and the trade-offs that you’re aware of. This can reduce the number of questions a reviewer might have about the code as they don’t need to make assumptions on what you have considered.

Some people don’t like annotations because the code should be “self-documenting”, it should be clear from the code what it’s doing. And I agree, but the code should say what it’s doing, whereas annotations could say why you chose to do it that way.

Q: How do we prevent obvious rule breaks?

This is where automation comes in. There are lots of static analysis tools which can check things like formatting, common bugs, using wrong idioms, using the wrong language in the wrong place (e.g. a language you might use for testing making it into production code).

Q: Should a junior developer also be the gatekeeper when he is doing the review? They may lack experience.

‘It depends’, of course. If the criteria for passing the review is “even the juniors can understand the code”, then, of course, she/he can be the gatekeeper. But usually, a formal gateway review is set up precisely because it needs to be a senior person to say yes to the code.

A combination may be possible – if juniors, or the whole team, or some subset, have looked at the code, made suggestions, and improvements have been made, then the gatekeeper (presumably a busy senior person) needs to do far less work as they only need to make suggestions based on things others haven’t thought of yet.

Q: Is there a potential for power imbalance in the gateway review model (e.g. indirect discrimination)? Do you have any ideas on how to tackle this, if this is the review model used?

Yes, there is definitely a potential for power imbalance. And this is where some of the anti-patterns come in, if the gatekeeper is always looking for problems, looking to say no. One way to get around this is to have more than one reviewer so they can keep each other honest, or you can rotate reviewers, so the burden is not just on a single gatekeeper. It’s also important that gatekeepers have their code reviewed too, to help them understand what it’s like to be reviewed.

We often think that it’s the gatekeeper’s responsibility to go through all the code meticulously, find every potential error and make sure everything is fixed before approving. I believe, however, that the gatekeeper should, instead, be checking that the code has been checked (by the automated tools, by the author’s peers) and that there’s nothing obviously wrong with it. Their job should be to approve it, not block it.

Q: How do you deal with an aggressive/unhelpful code review?

Having clear objectives for the review and the review process should a) prevent unhelpful code reviews and b) highlight when a reviewer is being aggressive or unfair rather than following the objectives. Also having more than one reviewer may help (see answer to last question).

There is a problem if the person who sets all the rules is also the person who does all the reviews. Under these circumstances, you may have to sit down with that person to let them know your concerns and get some clarity over the review objectives, or you may even need outside help or to speak to that person’s manager. Remember that a code review should be helping code to get into production, not preventing code from being used by your users, you may need to remind an unhelpful reviewer’s manager of this fact.

Q: Who should explain/show the code? The one that wrote it or the one that does the review?

I had assumed the code author would walk through the code and show it to the reviewer, but actually, you raise a good point. There’s definitely a use case for the reviewer to walk through the code in front of the author, to see if they’ve understood it correctly. I think I would suggest that if a reviewer is less experienced, it might be interesting to have them walk through the code to the author so they can check if they’ve understood it correctly. This might be a good way for code authors to see how their code is read and understood.

Q: It’s there a specific format/tone of the messages for code reviews? Any things to be avoided?

Yes, you should be nice! I think a good basic guideline is don’t say something you wouldn’t say to a person’s face. When writing comments, try to think about the person who’s going to read them (and how they might “hear” it) and when reading comments try to bear in mind that the commenter is (probably) trying to help and may have been short on time when they wrote it.

Be constructive, be specific. Be clear what your issue/question is, be clear on what you want the reader to do – answer the question; make a change; don’t make a change but learn for the future etc.

How to do you do code reviews on a big project with no automated tests?

If you have no automated tests, then you really need code reviews! In fact, a project like this can use a code review process to focus on the things you want to change and make those changes with every bug fixed and every new feature. E.g. every code review should have an automated test suit, and some tech debt addressed (e.g. maybe separating business logic and infrastructure code).

It can be difficult to sell the business on this, as you might find these code reviews start to slow down the process. If you can try to do the code reviews for a period of time (maybe 3 months?) you should start to see an improvement in quality (e.g. reduction in bugs) that will help people buy into continuing with the code reviews.

Q: How do you include remote workers in a pair-programming like experience?

I think you have to settle for some really excellent screen-sharing tools, particularly with the ability for either user to take control of the cursor. Without being able to share control, you’ve basically got a driver and a bored navigator.

You could also try a really light-weight code review process which is similar to pairing, like maybe reviewing every half day. This isn’t the same thing as having conversations at the desk, of course. If anyone has alternative answers to this question I would love to hear them in the comments

]]>
https://blog.jetbrains.com/upsource/2018/10/16/webinar-recording-code-review-best-practices/feed/ 0
Live webinar: Code Review Best Practices https://blog.jetbrains.com/upsource/2018/09/20/live-webinar-code-review-best-practices/ https://blog.jetbrains.com/upsource/2018/09/20/live-webinar-code-review-best-practices/#respond Thu, 20 Sep 2018 13:14:40 +0000 https://blog.jetbrains.com/upsource/?p=1609 We’ve recently published a screencast highlighting some of the best practices teams may want to keep in mind when approaching code reviews. We’ve also shown how Upsource can help you follow these practices. Given how broad this topic is, we feel it deserves more attention. This is why we’ve decided to host a live webinar in which we’ll dive deeper into code review best practices with Trisha Gee.

Join us Wednesday, October 10th, 16:00 – 17:00 CEST (14:00 – 15:00 GMT / 10:00 AM – 11:00 AM EDT) to learn how your team can make code review process more efficient.

Upsource webinar: Code Review Best Practices

Space is limited, please register now.

About presenter:

Trisha_Gee
Trisha has developed Java applications for a range of industries, including finance, manufacturing, software and non-profit, for companies of all sizes. She has expertise in Java high performance systems, is passionate about enabling developer productivity, and dabbles with Open Source development. Trisha is a Java Champion and is a Developer Advocate for Upsource and IntelliJ IDEA.

]]>
https://blog.jetbrains.com/upsource/2018/09/20/live-webinar-code-review-best-practices/feed/ 0
Code Review Best Practices https://blog.jetbrains.com/upsource/2018/08/30/code-review-best-practices/ https://blog.jetbrains.com/upsource/2018/08/30/code-review-best-practices/#comments Thu, 30 Aug 2018 08:04:41 +0000 https://blog.jetbrains.com/upsource/?p=1599 We’ve created a new screencast outlining some of the best practices that apply to performing code reviews, and how Upsource can help apply those best practices. In this blog post we’ve also transcribed the content, and have provided links to further information.

https://youtube.com/watch?v=EjwD7Pi7J_0

1. Automate everything

[starts at 0:05 in video]

Firstly it’s important to automate as much as possible. It’s best to save the valuable time of your human reviewers by using tools such as continuous integration servers like TeamCity to ensure the build compiles and automated tests pass. Upsource supports integration with other tools, it can do things like show the results of a build for each commit, so it’s easy to see there’s no need to review commits where the build failed.

Code reviewers should never have to worry about whether code compiles or passes easily automated criteria. Upsource provides code intelligence for Java, Kotlin, JavaScript, PHP and Python. This means that when a reviewer is looking at code in Upsource they can see red or yellow warnings if the automated inspections find problems in the code. Upsource also integrates with external inspection tools like SonarQube.

As well as integration with build servers, Upsource also integrates issue trackers, like Jira and YouTrack, so it’s easy to navigate to the issue containing the details of what these changes are supposed to achieve.

Upsource also lets us automate a lot of our code review workflow, for example creating reviews and assigning people to them.

2. Agree on Code Review Goals

[starts at 1:02 in video]

If we’ve automated as much as possible to determine the quality of our code, we need to decide what’s valuable for our human code reviewers to be looking.  Even taking away the checks that can be easily automated, like compilation, formatting, unit and system testing, there are still many different aspects of the code that a reviewer could be looking at. Selecting the important ones to check will depend upon the team and how and when they review code.  For example, reviewing the design of a large feature right at the end of the feature implementation is either too late to make changes or could significantly delay the release of that feature.

The goals for the team might be to:

  • Make sure multiple members of the team understand this new piece of code
  • Check the design follows the application’s standards
  • Ensure the quality of the code. This should involve not only checking the presence of automated tests, but also whether the tests are testing the right things
  • Identify potential security issues
  • Use code reviews to collaborate early to find the right approach or design, and iterate over the development.

Whatever the goals are, the team needs to identify them early and apply them consistently. Note that many goals may be mutually exclusive, so it’s not possible to check for everything in every code review.

3. Submitting for Review

[starts at 2:15 in video]

Assuming the team has a set of goals for code reviews, a developer is going to want to submit their code for review.

Upsource lets a code author create several types of reviews manually, and can even automate the creation of reviews.  A developer can choose to add a commit to an existing review, to create a new review from a single commit, or to create a review that tracks a whole branch – this last option will automatically add all new commits on this branch to this review.

We need to select reviewers for this review based on whatever our team guidelines are. This is going to depend upon the goals of the review – if the goal of the review is to pass some sort of gateway or quality check, there’s likely to be an individual or group of specialists who should check the code. If the goal is to share the details of the implementation with the team, the reviewers could probably be anyone, or maybe even everyone, on the team. You can also select watchers, which suggests that these people should be aware of these code changes but are not required to comment on them.

Upsource can make the selection of reviewers easier. You can configure Upsource to automatically add reviewers or groups of reviewers based on certain criteria, such as the type of review and the author of the code.  Upsource can also automatically suggest reviewers based on past review history.

Code authors can help reviewers to understand the code and the decisions taken while writing it by annotating it with comments. Note that by leaving comments in Upsource, rather than the code, the comments are potentially short-lived. The intention is that they only live during the review period, and their purpose is specifically for helping reviewer understanding. Although we’ll see later that comments in Upsource can live outside of the context of a review, as a code author we’ll generally use them to communicate our thoughts to a reviewer.

It’s good practice to also label the comments so it’s clearer what purpose the comment serves.  We’ll see more of this later.

4. Reviewing Code

[starts at 4:09 in video]

Now let’s look at best practices for reviewing code. The most important thing for a reviewer is to review the code as quickly as possible. The code author is likely to be waiting for the results of the review, and the longer it takes to receive feedback the harder it will be to remember the context of the changes.

Upsource shows the reviewer whether these revisions pass the automated build, so if this is green it’s reasonable to assume we can go ahead and review the code.

The issue tracker integration here lets us see at a glance the summary of the bug or feature being addressed by these code changes. This helps us as a reviewer see what problem the code is trying to fix, and reminds us to check whether the end result is what was actually needed.

Upsource also shows us whether the code author is online right now, if they are it’s probably a good time to review the code as it’s more likely that the author will respond quickly to any questions or comments.

We can use reactions in response to someone else’s comments as a shortcut to show we’ve read them and understand or agree (or not).

We should be writing our own comments about the code near the relevant sections of code. Feedback should be constructive and comments should be about the code, not personal about the author.

It’s important that a reviewer labels each of their comments with the relevant tag, so the code author can easily see if this comment is a showstopper, a question that needs answering or maybe a nice-to-have, otherwise an author may be unclear about what to do to address the comment, or if it even needs to be addressed.

Code reviews can be difficult for code authors, as we developers can be attached to our code. It’s a nice idea to also leave positive feedback on the reviews as well as noting changes that need to be made. This might not seem like an important step in terms of getting great code out of the review, but it is an important step in order to motivate developers and perhaps overcome some of the fear or dislike that some people feel towards code reviews.

We may also decide to come back to some of the changes later for whatever reason, a good way to help us to remember where we were and best represent our progress is to mark a file as unread if we mean to return to it.

5. Iterating

[starts at 5:50 in video]

Code reviews are naturally iterative, even the best code should elicit comments to be read.  Upsource shows us what state our review is in, in this case it’s “Open” which means the reviewers are still in the process of checking the code.

As the code author, may want to use Upsource’s progress view to see how much of our code a reviewer has checked. We can also see whether the reviewer is currently online, and if so this is probably a good time to ping them directly via a review-level comment to ask them politely if they can finish the review so we can make any required changes.

When as a reviewer we’ve finished checking all the changes, it’s good practice to indicate that we’ve made all the comments we’re going to make. In Upsource, we do this by Accepting the review, if we’re happy with the code as it is and don’t require any more changes, or Raising a Concern, which means that as a reviewer, we expect the author to answer our comments and possibly make changes to the code.

Upsource shows the results for each reviewer, with either a purple face for those who’ve raised concerns, or a green smiley face on the icons of those who’ve accepted the review.

As a code author, if there are concerns that need addressing we should fix these as soon as possible, in the same way that reviewers should review code as soon as possible.  While it can be painful to context switch between tasks that one may be working on and another task like a code review, it’s less painful if there is less time between iterations of the review, it’s easier to remember the context if there isn’t a span of days or even weeks between writing the code and making changes.

If a review is based on a branch, as soon as we’ve committed a new change to the branch it’s automatically added to the review, and, of course, our build server compiles and tests the code once it’s checked in.

To make it easier to see which comments are still relevant or outstanding, it’s a good idea for the person who started a discussion to resolve it when there’s no further action to take. So in this example, as we’re the code author and we can see that the reviewer has read our explanation comments, we can now mark those as resolved.  To show only the outstanding discussions we can then hide the resolved discussions from the review, or even filter by label.

When we change code in a review, we should respond to the comments the reviewer made. We can either write full responses or use a reaction to acknowledge the point. We should also resolve any discussions we started that don’t need further action.

Once again, it’s a nice idea to annotate the code with comments, questions or ideas so the reviewer understands the thought that went into the code, or maybe to ask for suggestions.

When changes have been made to the code in a review, we can look at it again as the reviewer.  Adding new code to the review resets the state for the reviewers, so they have to once again select whether the code is Accepted or if they’ve Raised a Concern.  Upsource also resets any files that have been changed to Unread status, so as a reviewer we know that we only need to look at the files that are unread, all the other files as the same as last time we looked at them.

When we’ve resolved discussions that don’t need further action, and we don’t have any other outstanding issues with the code, we can accept the review.

6. Closing the Review

[starts at 9:13 in video]

Probably the most important part of the review is understanding that when code is good to go and closing it. Without this step, the code the author has worked so hard on is living in limbo and not delivering any value to anyone.  Once again, it’s important the team has decided in advance the criteria under which all reviews are considered closed – should it be when all reviewers have accepted it, or some subset? If it’s a subset of reviewers, is it important which individuals accept, or is it purely a number, for example at least 2 out of 3 reviewers? And what do you do if one or more reviewers have raised concerns, do they all need to be addressed, or can some reviewers be overridden by experts or by a majority?

Whatever your team decides, these standards should be applied consistently across all reviews.  Upsource is flexible enough to allow any reviewer or author to close a review whenever they want to, which means it’s down to them to apply the rules decided by the team.

Once a review is closed, this is probably the time to merge or publish our changes – again it’s up to the team to decide when this is done and by whom.  If the project uses Upsource’s integration with GitHub, the code can be merged via Upsource itself.

Closing a review doesn’t necessarily mean that all the discussions go away.  Discussions that have not been resolved live on in the code, so if we come across this code later we can see these unresolved comments, and it might be at that time that we do something about them. For example, we can use them to track possible tech debt or potential refactoring.

In Summary

[starts at 10:34 in video]

We’ve looked at some best practices for code review and how to apply those inside Upsource.

It’s important to automate as much as possible.  Ideally your code review tool will show you the results of automation performed using other tools, such as a build server. Upsource also provides the ability to automate a lot of the code review workflow, and also has code intelligence for Java, Kotlin, JavaScript, PHP and Python so code reviewers can focus on the things only human reviewers can do.

The team needs to understand what’s the purpose of their code reviews, and apply standards consistently across all the reviews.  This might mean having a checklist of things to look for in reviews or it might be a set of rough guidelines. It also means knowing who is responsible for reviewing code in which sections of the application, and stating how it’s decided that a code review is complete and the code can be merged or published.

During the review it’s important to respond in a timely fashions as both a reviewer and an author, to minimise the cost of the context switch between whatever they’re working on and the code under review.

It’s also important as a reviewer to be clear about what you expect the code author to do in response to comments – should the code be changed or is it merely a comment to learn from and apply in the future?

Most importantly of all, the goal of a code review is to have the code pass the review, and make it into production. Code under review is usually code that’s not being used, and code that’s not being used is not adding any value to the application or the users.

]]>
https://blog.jetbrains.com/upsource/2018/08/30/code-review-best-practices/feed/ 10