Code Reviewer Spotlight: James Slater

Code Reviewer Spotlight: James Slater

PullRequest reviewer James Slater learned Google’s code review process by doing. Over several months of demonstrating his understanding of the style guide, code clarity, language mastery and best practices, he was finally granted JavaScript Readability at Google — an internal certification that shows you understand Google’s best practices and coding style for a given language.

images/james-slater-small.jpg

Completing the demanding readability process showed James' commitment to improving code quality and mentoring others, and it also highlighted what he liked and disliked about the code review process. Over his engineering career, he found the right balance of positive, constructive guidance, and now provides code review as a service with PullRequest.

We interviewed our top reviewers and asked them the following seven questions:

1. What’s your background?

I started as a sound designer for video games, then moved into software engineering at creative agencies, like AKQA, R/GA’s embedded G Team at Google, and Idean. I’m currently a senior software engineer at Capital One.

2. Why is code quality important?

The first principle of understanding what code is, is that it’s a language. You wouldn’t write a story that’s indecipherable or missing key story beats. Code is no different.

Like a story, code is read more often than it’s written, and other people need to be able to read and understand it for it to have any meaningful value. I try and provide mentorship through my comments to help encourage developers to write high-quality, readable code.

3. Why is a good code review process important?

When I look back on my own projects, I want to be able to see why I made these specific decisions and understand what these different functions are designed to do. Even though I wrote it, and I may have written the code just a few months earlier, I probably won’t remember. I lean on my annotations and comments to help provide that context.

Now, imagine working with a team of developers. The person reading the code today might not be the same person who originally wrote it. It becomes even more critical than to write readable code that anyone can look at and understand the decisions that were made, and why.

4. Why do you review with PullRequest?

On previous jobs, I’ve been on the receiving end of some bad reviews where the reviewer was as curt and negative as possible, to the point where I actually felt like quitting. No one should ever have to feel that way. Instead, I strive to be constructive and positive, and I really try and help developers write the best code they can.

Also, I enjoy seeing all of the interesting projects people are working on. With PullRequest, I’m exposed to so many different companies and developers, and I have fun reviewing their code. I get to offer my help and stay updated on what’s trending — it’s win-win.

5. What advantages have you found of being a third-party reviewer?

When I worked on getting my JavaScript Readability at Google, which is a challenging and time-consuming accomplishment, I was so focused on iterating through a loop event and got caught up on the complexity of some very specific details. My reviewer was able to take a step back, and asked why I wasn’t just using a sort. I needed that fresh set of eyes, and having his objective feedback helped me better articulate what I trying to achieve. My end result was better because of their feedback.

I see my role as a PullRequest reviewer as largely the same. I’ve been on the other end, and I know how easy it can be to miss what later seems so obvious. Everybody’s on a deadline and wants to ship to production, no one wants to be a blocker, so I leave comments that shouldn’t add overhead and are meant to help improve code quality over the long run.

6. What are some common issues you see?

A little bit of annotation goes a very long way. At Google, it’s required to include a file overview at the top. Depending on the project or the team, that might be overkill, but annotating what this function does and what these arguments are, helps ensure other developers can continue to maintain the code over time.

One of my tactics is to ask developers what a particular function does. If they struggle to answer concisely, it might mean the function is too long and can be simplified, or maybe just a short annotation is enough to provide that necessary context.

7. What advice do you have for developers?

Some engineers make their review comments as terse and short as possible, but that brevity can create confusion, or be misinterpreted as rudeness. I know that’s not their intention, they’re just trying to be brief, but I’ve found that my taking the time to write quality comments that are kind and also concise really pays off. It reduces miscommunication and reduces latency in reading review feedback.


About PullRequest

HackerOne PullRequest is a platform for code review, built for teams of all sizes. We have a network of expert engineers enhanced by AI, to help you ship secure code, faster.

Learn more about PullRequest

Brennan Angel headshot
by Brennan Angel

December 10, 2018