8 February 2017, by Harry Cummings
This is part of a series of blog posts on code reviews, based on two sessions of an internal discussion forum at Softwire. See the first post in this series for more information. In this post, we’ll discuss improving the interaction between the reviewer and the recipient of the review (i.e. the developer).
How do we mediate reviews?
There are all sorts of ways to mediate a review:
- Specialist review tools like Crucible and UpSource
- Repository systems with review features, like GitHub, GitLab, and Gerrit
- Sending diffs/patches over email
- Talking through the code face-to-face, e.g.
- Have the reviewer sit with the developer at their computer
- Reviewing code as a team around a big screen
- The old-fashioned way: Print out the code, stick it to the wall, and scribble over it together
- Pair-programming is arguably a form of code review, taking place as the code is written
Broadly speaking, all of the above approaches fall into two categories: asynchronous online reviews, or in-person reviews. Which kind of approach people preferred was probably the most contentious part of our discussions at Softwire. There was a range of opinions here, highlighting benefits and drawbacks to each approach.
Asynchronous online reviews vs. in-person reviews
Several people made the point that in-person reviews can be more useful for training, and for knowledge sharing in both directions. Face-to-face discussions make it easier to provide context around the code changes. They also give the developer a chance to talk the reviewer through the changes in a sensible order, or perhaps commit-by-commit. This might be better than the arbitrary order in which changes are presented by a diff/patch or an online review tool.
In-person reviews may also provide opportunities to pick up other context that might not be directly relevant to the code quality but is useful for the reviewer to know. For example, any frustrating obstacles the developer encountered while working on the task, which the team might need to address. Reviewing in-person can also save developers from context-switching. If you have enough reviewers on a team, developers can get a review as soon as they finish their code rather than starting on another task and subsequently switching back to deal with review feedback. This obviously comes at the cost of the reviewers having to make themselves highly interruptable though.
A lot of the literature on code reviews also favours some kind of in-person reviews. Here’s one particularly strongly stated example:
“Effective code reviews are not done via a tool or remotely—they are done when you’re sitting side-by-side with the person or pair who just wrote the code. This personal way allows you to share and teach much more information than you can pass in a text-based tool. Don’t skimp on this! If you’re going to do code reviews because your code sucks, do them right.” – Roy Osherove in Notes to a Software Team Leader
On the other hand, some of our reviewers felt that asynchronous online reviews were better partly because they don’t provide the reviewer any additional context. Online reviews arguably make for a more authentic review of maintainability (future developers on the project probably won’t have the luxury of talking through the code with the original developer). Also, coming at the review from their own angle might allow the reviewer to spot issues that the developer has missed.
One major advantage of online tools is that they leave a permanent record of review comments. Some tooling combinations make it particularly easy to go back through old reviews (for example, Crucible with JIRA integration). Several people had worked on projects where they had benefited from the ability to do this.
Several people found it useful to mix online and in-person approaches, perhaps depending on the nature of the change. For example:
- Performing a high-level review in isolation first, then talking through with the developer for context, before finally performing a line-by-line review online.
- Saving face-to-face reviews for bigger or more complex changes
- Carrying out most of the review discussion in person, but using an online tool to track this. That is, initiating the review process and documenting any important outcomes of the discussion.
Reviewers: Making code reviews better for the developer
Quite a few people found phrasing review comments to be a challenge, especially when using online review tools. Some of our reviewers were concerned whether we did enough to make new-starters comfortable with the process, and to make it clear that they can and should challenge their reviewers. After all, the developer is always closest to the code and knows it best. It can be worth a reviewer (particularly one in a more senior position) reminding the developer of this explicitly.
Ways to make reviews more positive included:
- Phrasing review comments as questions or suggestions rather than statements
- Talking through major issues in person rather than writing lengthy review comments
- Talking through and fixing batches of very minor issues in person, rather than writing lots of tiny review comments
- Remembering to always make some positive comments (especially in reviews with some criticisms elsewhere)
This might be more or less important depending on the developer. Generally, reviewers should be conscious of the recipient of the code review and tailor things to them. Extra tact may be required when reviewing code from external third parties, which can be politically awkward.
Developers: Making code reviews better for the reviewer
This wasn’t a question we had set out to answer with our discussion. However, people naturally mentioned how they approached submitting their own work for review, and several common points arose:
- Performing their own review of the changes first (ideally in the same review tool the reviewer will be using)
- Linking to any relevant context (e.g. the relevant ticket in the project’s issue tracker)
- Keeping auto-generated files out of the review (if appropriate and the review tool allows)
- Splitting into sensible commits
- Especially keeping big renames, file moves, or other refactorings separate
- Also splitting code changes across multiple commits where appropriate)
- On one project, we experimented with commiting and reviewing one test at a time, resulting in many small reviews. On a small team, this turned out to be a very effective workflow.
As we saw in the previous post, there are many different valid approaches to code reviews. Organisations should give teams the flexibility to choose a code review process that meets their needs. The first post in this series covered the wide and varied benefits of code reviews. As a team, you should reflect on your code review process, considering what value it provides and what further value it could provide. This will allow you to evolve your code review process to be more effective. I hope this series gives you some ideas that you find useful. Please feel free to share your own ideas on code reviews in the comments below.
1 February 2017, by Harry Cummings
This is part of a series of blog posts on code reviews, based on two sessions of an internal discussion forum at Softwire. See the first post in this series for more information. In this post, we’ll cover some of our current approaches to code reviews.
We tend to at least implicitly perform code reviews in multiple passes. These break down into three stages:
- “Outside-in” preliminary review
- Reading through the original user story or defect
- Reviewing the design
- Checking out the dev branch and doing some cursory testing (this can be useful for reviewing UI issues or things that are hard to spot from the code or by automated tests)
- Reviewing the tests at a high level (do they function as good developer documentation for the code)
- Review of the code itself and the tests in detail
- Review of any activities surrounding the code change, e.g.:
- Manual testing
- External documentation
- Risk/impact assessment
Note that not every project needs all of these passes. The point is that “code review” is a broad term covering a range of activities. Which activities you carry out, and when, may vary by project. Although within each project, there’s a lot of value in being consistent. Consistency helps developers become comfortable with the review process, and makes code reviews a much more reliable tool for quality assurance.
When do we review
As noted above, different code review activities may be carried out at different times. There was a general consensus in our discussions that reviewing earlier is preferable. Most projects insisted on at least some form of review before commit, although a few relaxed this in special cases (depending on the type of project) to avoid becoming a bottleneck.
About half our teams are actively performing up-front High-Level Design reviews. These can be useful for everyone but especially for less experienced developers (which might just mean less experience with the particular project). They encourage working through design issues up front, avoiding wasted time at the implementation stage. It also means the code reviews can then focus on just the code. The only problem mentioned with HLD reviews was that it can be a bit unclear what we mean by an HLD, and sometimes people go too low-level. For projects broken up into well-sized tasks, an HLD could just be a couple of sentences and a few bullet points.
An alternative to up-front HLD reviews is reviewing roughly implemented code, essentially a spike or proof-of-concept. This can be particularly useful on tricky legacy codebases, where it might be hard to see how to go about introducing new functionality.
Who carries out reviews?
Most of the people in our discussions were their team’s technical lead. Unsurprisingly, tech leads were doing reviews themselves, but there was a lot of support for reviews being done by not only the tech lead. Getting more people involved in the review process is a good way to build people’s confidence, share knowledge within the team, and help people become more comfortable with the review process. One person doing all the reviews can also become a bottleneck and slow the team down. Perhaps more importantly, giving developers the autonomy to carry out genuine peer reviews is a show of faith in the team’s ability, and makes it easier for reviews to act as a positive motivator.
One problem with having multiple people involved in reviewing is that it can become confusing for developers. It’s not always clear how to pick an initial reviewer, or when a review would be considered “done”. It’s important for each team to agree on a consistent approach, although approaches can of course vary between teams. Most of our teams use one of the following approaches:
- Let the developer choose the initial reviewer and allow the developer or the reviewer escalate to the tech lead if needed
- Have a high-level second line review as part of the standard review process
- Include the tech lead on every review, but allow developers to merge their changes as soon as at least one person has reviewed it. This prevents the tech lead becoming a bottleneck but still gives them a chance to go into detail on any red flags.
All the above approaches include the possibility of the tech lead acting as a second line reviewer. Our tech leads would go into more or less detail in their review based on the nature of the change and the experience of the other people involved (i.e. the original developer and reviewer). In some cases perhaps just reviewing the comments from the first-line reviewer and/or looking out for changes within common problem areas of the codebase.
How much detail to go into in a second line review is a matter of judgement and may not be obvious. It can help to think of the goal of reviewing as gaining trust that the code is up to standard, and getting involved enough to meet this goal. Of course, it’s still worth bearing in mind the importance of code reviews for training and mentoring. A second-line reviewer may be looking out for learning opportunities for both the developer and the initial reviewer. They’re also in a position to assess the quality of the interaction between these two roles. This will be the subject of the next post.
25 January 2017, by Harry Cummings
There is a lot of writing on the importance of code reviews and how to go about them at a low-level (i.e. what to look for in the code). This series of posts takes a higher-level look at how to approach code reviews and how to maximise their benefits. We’ll also refer throughout to other useful writing on code reviewing and how to make the most of it.
At Softwire, we have always carried out code reviews in one form or another. But our methods and tooling have evolved over time, and often varied between projects. This allows each project team to find a way of working that best suits them.
We recently ran couple of internal lunchtime discussion forums to talk about code reviewing and pool our collective experience. The goal here wasn’t to try and agree on “one true code review methodology”, but just to share ideas between teams. We discussed the perceived benefits and overall aims of code review, and how we go about achieving these.
It turned out that the approaches to code reviews within the company are even more varied than I’d thought. But there was some broad consensus on the benefits and aims of code reviews. However, the benefits that we valued most may surprise you.
What value do we get from code reviews?
Both of our discussions came up with a similar consensus on the range of benefits provided by code reviews. In rough order of their prominence in the discussions, these were:
- Training and mentoring
- In fact, several people felt that the healthiest approach to code reviewing was to treat it as a training opportunity first
- Knowledge sharing
- In both directions (i.e. both the developer and the reviewer had opportunities to learn from one another)
- This includes sharing domain knowledge, general technical knowledge, and knowledge of the specific codebase
- Encouraging people to produce better work (knowing that it will be scrutinised by your peers)
- Keeping the codebase consistent, in terms of style and structure
- More generally, keeping the codebase maintainable
- Catching defects, or at least catching them earlier (and so making them cheaper to fix)
One point I found interesting here was that “catching defects” was one of the last points to come up, in both discussions. There was a lot more emphasis on the holistic benefits of code review: training, knowledge sharing, and improving the overall codebase.
Quality assurance is the main focus of some widely-referenced sources on code reviewing. For example, Jeff Atwood’s blog post Code Reviews: Just Do It, and the two books that it references (Peer Reviews in Software and Code Complete). These mainly focus on reducing errors/defects/bugs, and only briefly mention other benefits. Of course, improving defect detection is not a bad argument for doing code reviews. Perhaps it’s also the most persuasive one in organisations that don’t have a history of doing code reviews and are reluctant to let people start spending time on it.
Defect detection is a valuable and somewhat measurable benefit, which may make it a good angle to sell the idea of doing code reviews at all. However, for the developers and tech leads who actually carry out code review, it doesn’t need to be the only aim or even the primary aim of the exercise. So how do we go about code reviews at Softwire? We’ll cover this in the next post.
6 December 2016, by Chris Arnott
What follows are three examples of how focusing on the future improves efficiency over focusing on the present.
Running — don’t look down
I often like to run at the weekend. I’m not sure what I’m running from, but I feel significantly more happy for the rest of the weekend if I go for a run on Saturday morning.
I’ve got some tactics to make sure I actually go for my run, as I’m very good at putting it off. Giving myself some accountability and telling my fiancée the night before helps, as she then keeps reminding me until I go out. I also find removing barriers to starting very helpful, by which I mean make sure everything is ready for me to head out the night before. This usually involves putting my running shoes by the door, and sleeping in my running shorts.
I’m not running with any particular aim at the moment, other than my happiness and fitness (although given the advice I’m giving in this blog post, perhaps I should have a more tangible goal to be aiming for), but I do use run keeper to track my run. This keeps me focused on my pace and encourages me to try my hardest. This also means I can notice when I’m speeding up/slowing down, to try and keep a consistent pace.
The main factors that I find, which affect my run are:
Hills are obvious. If it’s steep, I’m slower. Posture is perhaps less obvious, and I’m not going to focus on it here, but good posture aids breathing, and consequently speed.
Focus is the big thing here. I’m much faster if, rather than staring at my feet, I focus on where I’m running to. Unfortunately, the more exhausted I am, the more difficult this is, so I’m active in checking where I’m looking while I run. If I find my head drooping, and my vision staring at the floor, I revert it back to far away, and focus on getting myself there. This helps me reach that place, and means I get to pay more attention to my surroundings as I pass them by.
Projects — where is the project going?
I’ve spoken about this before in Don’t touch the patient, but I was mainly talking about tech leads in that post. This point applies more generally to everyone on a team.
You need to know where your project is heading, or your short term decisions will be in the wrong direction.
If you only focus on your current tasks, it’s easy to not spot things that will be an issue a week/month down the line. It’s important to spot these issues and resolve them early, as they are easiest to fix the earlier you address them.
To maintain this forward looking vision, it is important to know how your current tasks fit into the larger picture. What is the higher purpose of your work, and what can you do in the present to ensure that your work fits in with everything else going on. Working this out will involve communicating with other people to know what their aims are. Often other people will come to you first, but if they don’t, ensure you know who to talk to and make sure that the conversations are happening.
Work — where am I going?
At the highest level, this focus on the future applies to your whole career. If you only ever focus on what you are doing this day/week/month, then when you come to reflect on the past year, you’ll find that you missed the chance to take the opportunities provided to you to further your career and drive it in the direction that you wanted.
Zoe Cunningham has written an excellent series of blog posts about getting the career you want, which I would recommend to anyone who wants to know more:
There are lots of situations in everything you do to ensure your focus is in the right place. Your life will be easier overall if you make sure that you focus on the future, as it will allow your present tasks to be progress you to your goals. Without this anticipation, you may find that you have not ended up where you expected, with lots of work required to get back on track.
This post first appeared on Chris Arnott’s blog.
9 November 2016, by Jenny Mulholland
Have you ever wanted to work for the public sector, but didn’t think that as an SME you would have the same opportunities as a large supplier? Softwire’s experience as a software supplier to the public sector has shown us that this is not the case. In this blog post we explore why bespoke software developers, design agencies, cloud product vendors or freelancers should consider working for the public sector.
- Firstly, the Government has a genuine commitment to increase public sector spend with SME’s. The target for 2020 is 33% * this follows an upward trend of 25% in 2015.
- Even if you have no experience in the public sector this should not detract you from applying. Softwire had little experience in the public sector but we have recently been able to win multiple varied and exciting projects. Experience in the private sector is extremely valid along as long as you can demonstrate your expertise through case studies and testimonials.
- There are procurement portals which list all the current opportunities available. Some good starting points are:
Explore all the frameworks on offer http://ccs-agreements.cabinetoffice.gov.uk/
Freelancers, bespoke providers, cloud product providers and cloud consultants should head to the digital marketplace https://www.digitalmarketplace.service.gov.uk/ and to search for larger digital projects from across the EU: http://ted.europa.eu/TED/misc/chooseLanguage.do=
- In most cases the projects have been scoped and funded in advance of selecting a supplier, mitigating your risk. Requirements and budgets are usually stated upfront allowing you to assess which projects would be a good fit for your organisation and there’s a clear and open process for asking questions.
- The scope and range of public sector work is very diverse. It’s not just local authorities and large public bodies. Public sector includes research institutions, infrastructure providers, regulators and many more.
- There is a strong government directive for digital transformation and you could be working on projects using leading-edge technologies, methodologies and design techniques. The Government Digital Service manual explains and encourages best practice to ensure project success. https://www.gov.uk/service-manual
- Public sector projects have a strong social impact. Through digital transformation the Government wants to push the boundaries of technology to improve and make public services more effective and efficient.
- The government mystery shopper portal ensures that the procurement process is fair to SME’s. https://www.gov.uk/government/publications/mystery-shopper-results-2016
- techUK works with the Government on behalf of the industry and is an advocate for the needs of small and medium sized tech companies.
- With the Government digital transformation budget expected to be around £1.8bn next year – what are you waiting for?
7 October 2016, by Zoe Cunningham
The following is an excerpt from my new book, “Galvanizing the Geeks – Tips for Managing Technical People”. You can buy the full book on my website, here.
These are some of the books – not all of them aimed specifically at the technology sector – that I’ve found useful throughout my career.
28 September 2016, by Chris Arnott
There are certain standards that govern good design and user experience. But eventually we all need to do something out of the ordinary or sufficiently complex that it will be accompanied by documentation. I’m not talking about the comments you leave in code (that’s a whole other kettle of fish). I am discussing the big document you have to write, which explains the nuances of your API, and how best to use your shiny new touch gestures.
If you think it’s almost as boring writing these documents as it is reading them, then I say you’re doing it wrong!
If you read on, I’ll explain what you should be thinking about before writing any documentation, and how this will help you enjoy the writing process more, as well as ensuring the end user gets the most from documents you produce.
15 September 2016, by Zoe Cunningham
This is the second in my series of posts about how to get the career you want. Step 1 in my infallible all-purpose career plan is to work out what you want to do.
How many of us really know what we want to do? And does anyone have a good way to find out if you don’t?
13 September 2016, by Zoe Cunningham
This is the step that really sounds the simplest. You’ve made a plan, so now you just need to follow it. Don’t be fooled though; this is the step where it’s easiest for us to trip ourselves up.
How often have you decided that you will do something, and then found that you didn’t actually do it? I will tidy the kitchen. Doh! If you’re anything like me – all the frigging time!
There are two things that work against us here FEAR and DOUBT.
12 September 2016, by Zoe Cunningham
This is the third in my series of posts about how to get the career you want. Step 2 in my infallible all-purpose career plan is to make a plan that will get you to where you want to be.
Now I bet that almost all of you make plans to execute tasks and projects all the time. But somehow when you come to make a plan to achieve a big scary career goal, it doesn’t seem so easy. Sure, you say, I’d love to be Managing Director, but how on earth would I ever get there?