Using Code Reviews and a Culture of Feedback to Develop Engineers
Code reviews are essential for a company’s process improvement.
But they can also be a critical process for an individual’s career development and feedback.
We spoke with Rami Chowdhury about the unique way he approached code reviews while he was a Principal Engineer at Upside Business Travel, a D.C. startup revolutionizing business trips for small and mid-sized companies by giving them access to big company travel benefits.
Rami believes that Code Reviews can build a culture of feedback that helps engineers continually get better.
The conversation below has been edited for length and content.
What is the code review process at Upside Travel?
Like a lot of startups, we host our code on GitHub. We use the GitHub Pull Request process as an integral part of developing our software.
Typically, one engineer at a time will create a branch on GitHub. They’ll work on a particular feature or bug-fix on that branch and open a Pull Request. The Pull Request is the main forum for discussion of details for that feature. Once that Pull Request is merged, it ties into our ticketing system. The Pull Request status lets us know if the feature has been built or closed.
I’ve encouraged opening up Pull Requests early to my team. Before the feature is fleshed out or complete, open it early and make a note that it’s not ready yet. This lets us use the Pull Request as a forum for discussion. It shows work in progress so we can discuss fixes early on.
When I was in college, we had a Writer’s Workshop where you’d bring half-finished drafts to class. Everyone would collaborate, and you would get useful feedback and criticism. That kind of constructive criticism – the Writer’s Workshop mentality – is encouraged on my team and across Upside Travel.
There was a talk at PyCon a few years ago titled “Using Art Critique Principles for Code Review”.
The inspiration for the presentation came from the speaker’s experience in art school.
The presentation covered the need for constructive criticism and the importance of seeking improvement for your creative work from the community. Those principles inform our process at Upside Travel and the “Writers Workshop” idea.
We constantly provide feedback like, “Here are some good things about this code, and here are ways it can get better.” The focus is on solving the problem and developing the skills of the developer.
It’s an opportunity to guide more junior engineers through a better pattern or give them information about a problem they might not be aware of. For example, a performance issue or a better way to use a particular library.
Are there any specific tools you use when doing code reviews?
We’ve found GitHub’s Web UI tools to be very effective. In order to track our general process, we use JIRA as a bug tracker. It is tied with GitHub and the information is passed along so the discussion is in multiple places.
Does your team do their own code reviews?
There are two factors to code reviews at Upside Travel. We do our code reviews and that determines whether the code in question becomes a part of the application. But we also have a dedicated QA team to test behavior.
There’s a nuance there between good code and the state of the application/product.
When we do code reviews, it can be very instructive. We talk through the code to create a better state of a particular component. But if a bug or flaw affects another component or the application as a whole, that’s the type of thing the QA team would catch through a regression test.
Typically, the QA team is testing behavior, not code. They say “there is an error on this screen when you click on this button” instead of “this line of code causes this bug.” It’s on the engineer to then track down where the code is breaking. They determine if it’s a bug we can fix or if additional code needs to be added to prevent this issue.
We try to make sure the code we produce is bug-free. But there are many things in a code-review that make it easy to miss the forest for the trees and get lost in the smaller details. There are some higher-level application behaviors that we might not be seeing. That’s the incredible value of a QA team. They have that whole picture and they can test a feature against the whole picture and determine if everything is working appropriately for the customer.
When do you pass it off to the QA team?
Our set-up has a development, staging, and production environment. The production environment is customer-facing, so ideally everything that goes there should be in good shape. When something is approved on GitHub and we merge the Pull Request, a tool automatically takes the latest version of the code from GitHub and deploys it to our development environment.
We do our own testing in the development and staging environments, after which it is passed to the QA team.
This experience is common, based on what I’ve experienced in my career. I’ve never really encountered a QA team that’s really involved in the details of the code. I think a QA team who is in the details of the code would be less able to focus on the big picture.
Upside has a QA team, but members of the QA team are embedded in each product feature team. When a product team is working on a set of features, there is a QA team member in the daily standups and process meetings. This gives them the context they need for the bigger picture.
Other organizations have the QA team testing yesterday’s or last week’s releases. In that case, they’re always playing catch up.
In our model, with QA embedded in Product, they know what’s going on and see a step ahead of their duties.
Searching for a new Job? Send us your resume.
