Gert-Jan van der Wel

hi@gertjanvanderwel.com

Here I share what I'm up to and working on at the moment. I aim to update it every few weeks. If you want me to send you an email whenever I wrote something new, you can sign up here.

Pull Request Follow Up

work - 28 Jun 2018

About two months ago we started using Pull Requests. For those out there who are not software developers, a pull request is way to bundle changes in the source code of the software - usually for a new feature, but not always - to have it reviewed by someone else. We introduced pull requests (or PRs) for mainly two reasons, to improve our code quality and to share the knowledge about what has changed in the code among more developers.

After a while we noticed that there was some confusion around how a PR should be reviewed. Should someone glance over the changes and approve a PR without much discussion or should it be thoroughly studied and debated? So yesterday I sat down with my senior developers to talk about it and we found that the discussions around PRs were mostly around code formatting & quality issues, bugs and problems in the code architecture.

I think debating over coding styles is a waste of time. We should pick one style (per language perhaps) and stick to it. We use Rubocop to check our Ruby code and we will start using something similar for our JavaScript code soon, Prettier for formatting and JSLint for quality. If a PR has a bug in the code it should not be approved of course, neither when the code architecture is wrong. We will do more automated testing to find bugs sooner. Most of our backend Ruby code has automated tests already, but the our frontend JavaScript code has not. We made our first steps into using Cypress.io for that and it looks very promising.

Another thing we have learned is that having a discussion about the code architecture after something has been built is not the right moment, not the right moment at all. The discussion should happen before the actual implementation has occurred as it usually takes quite some time to change the architecture afterwards, wasting costly resources. So when building a new feature, a developer and the reviewer will sit together and talk about it before they start hitting their keyboards.

I think these improvements make a lot of sense and I’ll keep you posted on how they work out for us.