Category: Uncategorized

I attended the Lead Developer conference in London a couple of weeks ago. I enjoyed it and came back with lots of ideas buzzing around in my head. It’s a single track conference, which is good because you don’t have to make decisions about what to see and what to miss, but also you get to see some things you might not have chosen just based on the title. Many of the speakers have given longer versions of the talks elsewhere, or have written articles on the subject, so if particular topics are of interest it is possible to go and dig in further. You can think of it like a taster menu at a fancy restaurant.

I talked about some of the talks I had seen at our developer meeting on Friday. I couldn’t cover all of them (23 in total I think), so concentrated on a few that had particularly resonated. The full set of conference videos are available to view on YouTube, so go and check them out. Here are some details of the handful of talks I discussed with the team:

Alex Hill – Giving and receiving code reviews gracefully

Alex has written up a longer form in this blog post while the video of her talk is on YouTube.

This talk was about the psychology of code reviews and how to take that into account to get the best outcomes. People sometimes feel defensive about code reviews as it feels as if they are being criticized rather than the code under review.

She talks about dividing up code review comments into 4 quadrants along 2 axes: High vs Low Conflict & High vs Low Reward. The Low Reward, High Conflict things tend to be preferences like where to put brackets and so on. The best way to handle these things is to agree code format standards and automate them away. The Low Conflict things don’t cause problems between team members because they are non-contentious. Things like obvious bugs (in the High Reward area) and debug statements (in the Low Reward area). It’s the High Reward, High Conflict things that are tricky. She suggests considering Conflict Resolution Archetypes- Avoiding vs Yielding vs Competing vs Collaborating. We are aiming for collaboration and she has some suggestions on how to achieve that.

These include: Doing more pair programming and having more discussion before implementing a feature. Ensuring everyone reviews and is reviewed, so there is a level playing field. Using “we” rather than “you” or the passive voice to keep the whole tone of the review more neutral. Asking questions rather than making demands. Just being positive rather than negative or confrontational.

As the receiver of the review, say thank you and also think about how you think someone else would respond.

Adrian Howard – Points don’t mean prizes

There is a longer version of this, in video form from the ACE conference while the short version from Lead Dev is on YouTube.

Adrian works in the intersection between development, UX and product helping companies build the right things. This talk was about various dysfunctions he sees in the way people think about Scrum, Agile and requirements.

The default scrum model that people use is kind of broken. Someone comes up with the vision that everyone is heading to. Someone comes up with the user journeys to get to that place, that gets split up into stories. Those stories are given to the developers and everyone lives happily ever after. But that’s a lie.

Problems arise because the different stories are different sizes. So it’s hard to put them into fixed sprint-sized boxes or to get flow in a Kanban approach. So break them up into smaller ones and we get smoother flow.
Give those to the developers and we’re done. Again that’s a lie.

Stories focus on size and effort not on actual value. So we may have split
up the story and actually delivered little value. So think about:

• Bin – can we discard or postpone a story?
• Thin – can we deliver less and still get value?
• Split – can we break up a story and still get value from the pieces?

Once that is done, give the stories to the development team and we are done. Once again, it’s a lie.

The problem is that often the people who want to follow this approach don’t have the authority to make it happen.

Adrian recommends User Story Mapping as a way to get good stories and keep the big picture in mind. He particularly likes the book that describes it, because if you give someone a book, it has much more weight than just, “hey try this technique”. The output is a map rather than a flat backlog. People tend to do this at the start, but it’s best to keep refining. Some of the ideas of this approach are described in Jeff Patton’s blog post that predates the book.

Nickolas Means – Who destroyed Three Mile Island?

The final talk I discussed from Day One of the conference was about the nuclear reactor meltdown at Three Mile Island. I recommend watching the video of this as he is a good story teller and I am not going to retell it in detail here.

He first outlined the events that lead to the partial meltdown occurring and then discussed the ideas of the “first story” and “second story” as described by Sidney Dekker’s book “Field Guide to Understanding Human Error“. The “first story” is written with hindsight and outcome bias and generally seeks to blame someone for the results. The “second story” seeks to look at what happened through the eyes of those who were there and what they knew at the time. The idea is to start with the assumption that everyone was doing the best they could with the information they had at the time, so human error is never the cause of the event. This leads into the idea of blame-free post-mortems as a way to discover and fix systemic problems rather than seeking someone to blame.

Uberto Barbini – Legacy Code – Big Rewrite or Progressive Rejuvenation?

The first talk I discussed from the second day of the conference was this one about legacy systems. The video of this talk is on YouTube.

A legacy system is old, but it works and usually makes money for the company, or it would have been retired.  One of the options for dealing with such a system is to just keep patching it as changes are required. The downside to this is that the system slowly degrades as more and more changes are added.

Another option is the big rewrite. This rarely works out. The thing you are replacing was successful, so not as simple to replace as you might think.
The old system contains quite a bit of knowledge that can be lost in the transition. Finally, data migration is nearly always harder than expected

The best approach seems to be the “Strangler” pattern as described by Martin Fowler whereby the new application wraps the old one and then slowly replaces it over time. This has the advantage of showing results quickly and not requiring a risky “big bang” switchover.

Uberto Barbini has a similar technique which he calls “Alchemical Rejuvenation” – Turning legacy code into gold.  It has the following steps:

• • Seal with external tests. First of all you need some high-level assurance that the system is working after you make changes. These tests may be discarded later, once there is better testing in place.
  • Split into modules. Start improving the internal architecture to separate into logical pieces.
  • Clean the module you need to work in, adding tests as you go.
  • Repeat as needed

He had an interesting take on code quality – It’s not clean code, TDD, or patterns etc. Those are just tools to get code quality. The real test is if your application has been running for 10 years and you can still add features and fix bugs quickly, then you have high code quality.

Kevin Goldsmith – Using Agile to Build Inclusive Teams

The final talk I discussed was about using agile techniques to improve the way teams are run. The video for this talk is also available on YouTube.

He talked about using post-its to work with one of his reports to work out want they each expected of each other. Similar to the idea of the “Manager Read Me”

In similar theme he talked about mentoring a lead. Again, working out where different responsibilities lie. Is the manager keeping it, Does the manager approve it, Does the new lead inform of their decisions, or Does the new lead take full responsibility?

He also talked about improving team meetings. When it comes to making a decision he has two approaches: Polling – everyone gives their opinion, but in the end the manager decides. Voting – everyone votes. In the end the Manager has to accept and defend the decision. He talked about having a collaborative team meeting agenda in a shared Google Doc. For larger groups he recommends the Lean Coffee approach.

Finally he talked about having more inclusive meetings. The lead needs to resist talking as other people will yield to them. He also suggested having an observer who points out interruptions, people not getting credit etc. This role should be rotated though to avoid people not contributing.

Chris talked again about Release It 2nd edition.

Last time, Chris talked about “Creating Stability” – things that can go wrong, and how to prevent that.

The next section “Living in Production” is about how a system works in production. Part of this is physical (networks, IPs, etc.). There can be clock problems particularly with VMs. It covers “12 factor apps” – which we’ve discussed before in the context of microservices, coming from the microservices ideas, this is all about making the app not depend on things on the box.

We discussed “Stucco apps” – where if you install subsequent versions 1, 2, 3 of an app on a box, then there will be bits of version 1 and 2 left over – so the app isn’t exactly any of those versions. Instead, you should rebuild from scratch each time (you could use Nix and NixOS for this…). We also discussed configuration – getting environment-appropriate configuration onto each box.

We had a digression about ambient sound from services – like putting microphones in the JET torus – so you can tell whether the system is running normally. Because humans are good at recognizing unusual noises or unusual changes in noise patterns, this can let you pick up on patterns of behaviour that aren’t otherwise obvious.

We talked about setting up logging to demonstrate that the high-level goals of the system are being met. For example, in some systems, it might be really important if page loads have become slow, or users cannot log in, or if the number of purchases per hour has significantly dropped; and these are the important business needs rather than just whether a box is up.

We briefly discussed the merits of the Unix command “uniq -n” for monitoring services; for example to find the counts of unique ID addresses. This is very useful for spotting patterns in your logs.

When upgrading data in SQL databases, the upgrade path is typically straightforward – you migrate it all via a migration, or the apps don’t work. In NoSQL databases, there is no schema, there may be multiple clients using the data imposing their own restrictions, and so it’s not so straightforward. The author suggests a “trickle then batch” approach of first converting the high priority items, then after a while, converting all the other items.

We talked about API changes, and contract tests created by consumers, and versioning of APIs.

The final part of the book is about systemic problems – a grab bag of issues that didn’t come up elsewhere. Load testing scripts can be overly polite and well behaved, and then sites break when hit with real users that aren’t well behaved – so the load testing scripts should be more impolite. We talked about chaos – we’ve discussed chaos monkeys before, but there are various refinements to this idea. For example, a default “opt-in” for chaos monkeys, with the ability to opt-out if your service cannot tolerate chaos. Also, a “zombie apocalypse” – you send home a bunch of people, and see whether any of them are indispensable or not.

Tim talked about Flux, Redux and VueX, for managing state in JavaScript applications.

The big change between Flux and Redux was that in Flux there was one store per feature; in Redux the entire application state is stored in one central object. This makes it easy to serialize and debug. Changes to the store are made via a “reducer” – which is a function that takes the current state, and transforms it into a new state – similar to the way that state is managed in Elm. To help organize the state, then you can break it down into a set of smaller views of that state. Because you’re abstracting all the data into one place, then the components themselves can be simpler and dumber. This makes the components themselves easier to test.

VueX is a reimplementation of Redux, but focussed on Vue. While you can use Redux with Vue, it makes more sense to use VueX since it integrates better with Vue’s properties and events. The VueX dev tools have built in support for VueX, and they list the actions that are triggered at any point, showing when the state changes, and allowing you to step back before that. Also, there are libraries to serialize your entire state to local storage whenever there is a change, which makes it harder to lose state if you close your browser.

We discussed exactly what “state” means, and how much you want to have stored in the VueX state. It comes down to what is useful to you – you don’t necessarily need to store everything, just the things you care about.

Rich talked about a current project that’s using VueX; with a wizard that collects data from the client across multiple pages. This uses VueX to manage state.

We discussed when VueX was worth using. It adds flexibility, but it also adds complexity and means that you are distributing the code to manage state to more places. The recommendation from the Gitlab blog is to use VueX always. Tim doesn’t completely agree – it does add some extra boilerplate, but on the other hand it’s good discipline to make what your application does clearer. He’d use VueX for a new single page app, perhaps whatever the side. Rich on the other hand thought that it was more important to consider whether there is state on the client side that needs to be remembered between pages – so the wizard app needs that, but an app where the state is always synchronized with the server doesn’t need it.

Rich also mentioned a prefetch option in Vue Router, which can potentially make apps using the router perform more quickly.

This week, in our dev meeting, Chris talked about Release It! (second edition) by Michael Nygard. He’d read the original years ago, before “dev-ops” existed, and it was the first time he’d really thought about putting applications into production. It was an eye-opener 10 years ago, so he was excited by the second edition.

The book discusses stability patterns and antipatterns; then about things to consider in production; then about deployment; then about evolution of applications. So far, Chris has only read the first two parts of the book.

It discusses “Designing for Production” rather than “Designing for QA”; starting with a case study about how a small problem escalated into a huge problem. Some approaches are:

• “Testing for Longevity” – identifying things that will fail after X continuous days running, which might not be picked up by standard CI approaches
• limiting chain reactions between servers if there’s a failure causing domino effects (often by blocked threads building up)
• self-denial attacks like sending out mass emails that trigger everyone to look at your site simultaneously
• dog-piling, which is a bit like the thundering herd problem, e.g. if everything server is updated on a schedule exactly on the hour it causes peaks in load
• automated tools going out of control, so applying limits to them is good;

Positive approaches to avoid these are:

• timeouts
• circuit breakers – isolate your system from the remote system when the remote system is down, providing an immediate “fail” response rather than blocking and running out of threads
• bulkheads – e.g. reserving a few threads for the admin tool to fix the problems, or the operating system saving some disk space so the root user can fix out-of-space problems
• steady state – e.g. log files shouldn’t become infinite in size, they should be tidied up; temporary files should be deleted
• fail fast – do validation as soon as possible, if there are expensive operations that require data, then get that data upfront
• for websites, distinguish between 40x errors and 50x errors – a user entering bad data shouldn’t trigger a circuit breaker
• let it crash – if something goes wrong, let it crash and then restart it, but this requires other architectural choices like supervisors
• handshaking – typical REST services don’t have any handshake involved; sometimes it would be useful if they could return an HTTP 503 to their caller to tell it to slow down
• asynch approaches – a pull system can choose the rate at which it receives content
• shedding load – dropping load if it can’t take any more
• back pressure – telling your caller “back off, I can’t take any more work”
• governor – applying limits to the automation tool, so it can’t fire up 10,000 boxes immediately – if it’s outside the standard range of what it does, then it will do it more slowly

When building a system using an Agile approach, then you need to decide when to start applying these techniques – since they may not be expressed directly in user stories, and they may require significant architectural choices that are hard to refactor into an existing application. We discussed that some of these patterns are directly supported in newer frameworks, such as Akka HTTP; which has direct support for back pressure, circuit breakers and asynch methods. This makes the decision about when to implement them easier.

At our dev meeting this week, Rich talked about ‘Designing Data Intensive Applications‘, that he’d been reading, and Inigo talked about ‘Talking to your TV using Rust‘.

‘Designing Data Intensive Applications’, from O’Reilly, describes various tools and approaches for storing and processing data. It’s split into four sections. Section 1 discusses the basics of databases and how they store content, for SQL databases, NoSQL databases, flat file stores. It covers in-memory stands, b-trees, indexing structures, and the best approaches to use in various situations. Section 2 talks about distributed databases – clustering, transactions, serializing commits at a point in time so shared logs can work, and so on. Section 3 discusses distributed batch processing, using tools like Spark. The first three sections were all very interesting and informative. The fourth section was not as good – it covered the future of data, and was a lot woollier than the previous sections. Rich found the book on the whole good.

To talk to his TV using Rust, Inigo used a Google Home Mini, an RM 3 Mini IR blaster, an Open Source library to interface with the IR blaster, and some Rust on a Raspberry Pi. The Google Home part was setting up a Google Home App via https://developers.google.com/actions/building-your-apps – you configure a set of actions and intents online, and then assign a ‘fulfillment’ to it, which is a web service you make available via HTTPS. This part is fairly straightforwards – it requires filling in various web forms, and it’s easy to publish a test version of an app that’s only available via your own Google account.

To actually use this for a home application, you need to have SSL, so you need a domain name and a certificate. Inigo used Amazon Route 53 for the domain name, and Lets Encrypt for the certificate. There are various other services that will provide a domain name for a dynamic IP address for free, although the original dyndns is no longer free.

THe ‘fulfillment’ part is a web service that Inigo wrote that accepts JSON from Google, and then calls out to a Python library that interacts with the IR blaster. He wrote the first version in Scala, to get something simple working fast, and then rewrote it in Rust. He learned from O’Reilly’s Programming Rust book, and used IDEA’s Rust plugin. The experience was positive, although it was a bit tricky to get some of the memory management working correctly as a first-timer using Rust. The code is on Github at https://github.com/inigo/googlehome-remotecontrol.

Stephen recently read Effective Java, by Josh Bloch, and we discussed things that he picked up from it, particularly approaching Java from a perspective of much more experience in C# than in Java.

• When you have multiple overloaded constructors, instead use a static factory method on the class. We discussed the advantages and disadvantages of this.
• We discussed the preference for interfaces rather than concrete classes in Java vs C#. We discussed how this was probably influenced by type inference and type erasure. It may also be related to Java’s history with frameworks like Hibernate and Spring, that initially used lots of interfaces to support proxies before bytecode rewriting became common.
• Overriding equals and hashcode – it’s common in Java, but less common in C#. In Scala, case classes remove a lot of this complexity. We discussed what identity means for classes and objects, and how you express that in various languages.
• The difference between Exceptions and RuntimeExceptions is surprising from a C# background. We discussed the merits of checked exceptions – we broadly agreed that in an ideal world, checked exceptions were a good idea, but in practice it depends on library authors being competent and library users being competent. Sadly, this doesn’t happen. Checked exceptions also cause issues for lambda functions. There was some discussion that the way Scala does this is better.
• The use of external libraries – it’s a lot more common in Java, whereas .NET developers tend to use the CLR content much more frequently. We discussed the issues of transitive dependencies in Java bulking out your build. We also discussed how the ‘small number of large libraries’ assumption is no longer true in .NET Core – there are many different packages within .NET Core.

We discussed Effective Java a while ago in a previous dev meeting, too.

 

We had several lighting talks in the dev meeting this week.

Rich talked about WitAI – a language parsing web service that helps with building chatbots. He’s written a “drinksbot” in it, which we used via Slack before the dev meeting to order drinks. You provide it with a corpus of example sentences, and it learns from that.

Chris spoke on the Interplanetary file system. In typical direct file request, you might retrieve it synchronously from a single location. IPFS allows you to retrieve content from a set of distributed stores – essentially a peer-to-peer, torrent-like filesystem, that can be more robust and potentially faster than a direct file retrieval; because content is stored can be stored in a number of places.

Reece talked about antagonistic attacks on neural networks. Because neural networks (e.g. for image recognition) will typically work by perturb the input image to make the neural network recognize it as something entirely different – even though the change to the human might be imperceptible. For example – researchers recently made a model of a turtle (to human eyes) look like a gun (to neural network recognition) to illustrate this issue. This is a problem with neural networks generally – that the neural network may be using very specific things for its recognition, and it may be hard to identify these.

Stephen talked about teaching (very) young developers – like his daughter – to program. There is a lot of implicit knowledge and setup and background in a typical dev environment, and you want to get to working code as quickly as possible. HTML 5 and JavaScript are good for this. You should optimize for small victories, and concentrate on fast feedback loops.

Tim talked about Vue, the JS framework. You create reusable components that have an HTML component and some connected JavaScript that can control . It’s somewhere in the middle in terms of ease of use – it’s more complicated and powerful than Knockout, but simpler and more usable than React and Angular (and it can be used initially just by including script tags, and then extended to manage things like transpilation). We’re using it in a number of projects at present.

Rodney talked about “Where I’ve been” – specifically about the Nix conference that he recently attended. There is a new Nix tool for handling packaging and building in the Nix system, which has a simpler (command-line) UI. Another talk was about the security developments to the Nix project, such as having specific security experts who can getting security notifications from other projects early on; about automated scanning tools for scanning deployed Nix implementations for security holes. Nix in production was being discussed – e.g. for a company that’s doing cycle dock management in the Netherlands. Using Nix throughout an entire environment, from development through to production, brings many advantages. At Tumblr, Nix is used for testing of live SQL instance replication. Then there was a Hackfest – with spontaneous collaboration, and good work done on Nix cross-compilation.

In our dev meeting this week, Alex talked about chatbots. Bots are a massive shift in how we interact with computers – but to some extent they’re still a solution looking for a problem. Bots are conversation driven – and developers typically have never dealt with conversation! There’s a lot of existing support for English language bots that comprehend language reasonably – but there’s far less support for non-English bots. There are libraries and services to help with this – for example, Microsoft Cognitive Services provides endpoints that pick out from the users input what the user is asking, and even what their emotions are. However, these tools are probabilistic – they will say “it’s 90% likely that the user wants this”, rather than “the user definitely wants this”. Bots can get confused – because the bot will typically maintain state so it can refer back to earlier elements of the conversation, it may sometimes lose track of where it is – and needs to be reset. However – this is adding complexity to what is supposedly an intuitive interface; and users may not understand the underlying complexity.

You need to think about the platform that you’re deploying to, in terms of how you develop your bot and what capabilities you expect it to have. Slack bots, bots responding to SMS, Facebook bots – all have different characteristics based on the capabilities of the platform they’re running against. This also shapes what users expect in terms of level of communication – no-one expects lengthy complex sentences from an SMS bot, but they might do from a Slack bot. Discord bots have an expectation that they’re used for gaming, because of the assumptions of the Discord culture. Facebook bots have more in the way of graphics and buttons – so they’re moving to “Monkey Island” style user interfaces, whereas other bots are more similar to an old-style text adventure user interface or a Twine (and Twine is good for developing bots).

For bots, uptime is critical – so the bot should always be available. However, it’s not essential that they respond immediately at machine speeds – in some ways its better to pause a few seconds, because it may feel more natural, as well as helping deal with load. Cloud hosting is useful for bot development to help with this availability and coping with load. Also tracking user responses is important.

Given these complexities – why bots? It offers a high engagement rate, because it’s the same way of communicating as people are used to using with their friends. Bots can be proactive – if a user looks up an event like a concert, then the bot can prompt that the user might want to buy a ticket for it. Bots can be more engaging for users who are unfamiliar with technology. Bots are new and exciting – it’s immature and in flux, and there are no great settled ways of doing things, so it’s good for experimentation. There’s lots of space for exploration – like creating a Twitter bot that offers to sell wine to people based on their tweets of wine bottles. They are a new space – and we should think about ‘how can a bot benefit you’ – like using a bot to collect orders to buy drinks at the bar. Go forth, and make the next Flappy Bird of bots!

At our dev meeting this week, we talked about three different alternatives to JavaScript – Elm, ScalaJS and GHCJS. All three allow you to write code in a modern, statically-typed functional language, and then produce output in JavaScript that can be run in a browser.

Chris had previously talked about Elm a few weeks ago, in “Elm and Punishment”. Elm is a Haskell-inspired language, designed specifically as a web language that compiles to JavaScript, HTML and CSS. Chris has written a sliding blocks puzzle (source) in Elm as a learning exercise.

Inigo was inspired by Chris’s work in Elm to look at ScalaJS. ScalaJS is a plugin for Scala/SBT that allows standard Scala code to be compiled to JavaScript. It covers the vast majority of the Scala standard library (although it excludes a few areas like reflection and reading files from disk). It doesn’t convert JVM bytecode, only Scala sourcecode, so Java code first needs to be converted to Scala, and it’s not possible to immediately use standard Java libraries. Inigo also wrote a sliding blocks puzzle (sourcecode) and a minesweeper clone (source).

Rodney took a different approach – he implemented Flattris – a flat version of Tetris (source). Rather than using Elm, which is inspired by Haskell, he used GHC JS – which actually is Haskell (the Glasgow Haskell Compiler’s JavaScript compiler).

All three of the languages turned out to be very similar in approach and in results. All of them worked very well. The core languages (Elm, Scala and Haskell) are all more powerful languages than JavaScript. The generated JavaScript is bulkier than hand-written JavaScript would be (from 150kb for ScalaJS, up to about 1MB for GHC JS), but this still isn’t particularly huge compared to the size of modern web pages. All three implementations used a pure functional approach (this is standard in Elm and Haskell, and recommended in Scala), and the sourcecode for the two different sliding blocks implementations ended up remarkably similar. ScalaJS and Elm both emit sourcemaps that allow the generated JavaScript to be debugged e.g. by stepping through the Scala source directly in the browser, or via IDEA. All three were enjoyable to program.

We’re not planning to embrace any of the three as a core language we use at 67 Bricks yet, but we were pleasantly surprised by how well they all worked, and we’re certainly considering them for the future.

 

 

At our last dev meeting, we discussed code reviews. We do code reviews for all code on all projects, before code is merged to the master branch, and everyone is familiar with doing them, but we can always improve how we do things. So, we had a general discussion on what good code reviews are, and how we can do them best.

We initially discussed what one should look for in a code review. We agreed on some questions a code reviewer should ask themselves:

• Do you understand the change being made, and more importantly why it’s being made?
• Is the change itself good? Is it clear, are things well-named? Is it testable; and does it have tests? Does it introduce any non-functional issues such as security or performance problems?
• One should look at what’s been done, but also the context of the change. Is there code elsewhere that should be reused? Are there similar changes to make elsewhere? Is there now obsolete code that can be removed?
• Even if the change itself is fine, has the codebase as a whole become less clearer? Has a file or method become too big?

We talked about some of the benefits we have had from good code reviews, such as:

• Finding out better ways of doing things – such as being able to change a large chunk of procedural code into much more concise and clear functional code, and new ways of doing legacy things
• Improving consistency throughout the system, using common approaches such as use of optionals, and putting code in the correct classes, clustering similar methods close to each other
• Getting the benefit of other peoples’ wider experience – e.g. new people have new perspectives and approaches that they have from previous companies, that we can learn from
• Sometimes it makes sense for the reviewer to make changes and improvements to the code, rather than just handing it straight back to the original developer
• Identifying areas that are unclear or confusing in the existing system as a whole, and that need refactoring
• Helping standardize our practices within and across teams, so we can improve generally

As a reviewer, you are aiming to do a good code review, but as a coder, you also have a responsibility to help make the code review useful as well. These include:

• Asking for a design review before starting the task at all
• Making useful, atomic commits, with meaningful commit messages, rather than a big bulk commit of everything followed by a few “fixing this” commits
• Making tests clear and readable, and avoiding repetition in them, just as we aim to make the main code clear and readable
• Review your code yourself first, before you commit – stepping back from the moment-by-moment coding often helps you spot issues. This incudes the basics – check that the code compiles and relevant tests pass
• Asking for partial review of long tasks, rather than waiting until the whole task is complete. Frequent review and merge makes the process simpler
• For a long branch, being ready to fix issues on that branch quickly if a reviewer find any, e.g. by reviewing other peoples code or by doing short tasks, rather than jumping into another long task

Ultimately, it is the reviewer that is doing the review, and we discussed the ways that a reviewer can improve the quality of their reviews:

• Consider whether you are the right person to review the code – usually it’s good to spread reviews around the team, but sometimes you might want an expert in a particular area to review the code
• Being able to start the review quickly – so prioritizing code reviewing over starting on new tasks
• Providing all the feedback from a review at once, rather than in dribs and drabs
• Pointing out what’s good, as well as what’s wrong in the code – this is good for morale, and also encourages good practices
• Providing a rationale for feedback – never just “do this”, but “this is a better way because…”
• There can be multiple issues with a piece of code – sometimes you’ll be initially distracted by minor points, but might need to revisit the review once those are resolved to spot fundamental problems of approach
• Pointing out alternative approaches that might be useful in future, even though a change isn’t needed in the current review
• Avoid giving feedback that is impossible to apply – as a reviewer, think about how you might solve a problem yourself, and discuss the issues with the reviewee

And after the review, for all the items raised, each should be discussed and either resolved, or agreed to be not needed now, or a ticket raised for them to be fixed soon.