engineers_field_guide

A field guide for Spiff Engineers

View on GitHub

Getting Work Done

In order to make real progress on our application, we need to have a holistic understanding of the system. Over time we will find ways to divide our overall product into separate systems so that developers can understand just one piece, but in the early days our best bet is to understand as many of the pieces as we can. Code review and Pair Programming are the two main tools we have for sharing our understanding.

Pair Programming

Pair programming is not mandated at Spiff. It is a tool we sometimes use in order to get help when we feel stuck or need to understand something better. Sometimes we pair just because it’s fun or we haven’t had a chance to talk to someone recently. Several of us at Spiff use LiveShare for Visual Studio Code as a quick way to share an editor. This works great for pairing with people who are remote, working from home, or just to let each of you use your own key bindings, but share the code.

While pair programming pay attention for cases where your pair misunderstood something in your code. You are getting a chance to see through the eyes of someone who hasn’t already been thinking about this and 1 month from now that will be you. “What does this do?” is not a big problem, but “this works like this, right?” is a big red flag if they guessed wrong.

Code Reviews

All changes to Spiff codebases should go through a pull request step. Pull requests are a great time to understand what other people in the team are working on and how it might affect your work. They are also a great time to ask questions.

“I followed the pattern in UpdateUserMutation here, does anyone know why we do it this way?”

During code review we try to automate as much of the mundane feedback as possible. Code formatting standards should be enforced by CI, rather than engineers spending time debating about it. The person opening the pull request should write up a brief that explains why a change is being made. Does it fix a bug? Part of a new feature? Refactoring?

The person reviewing a PR should focus on questions like:

This type of code review can be time-consuming because you will need to understand the “why” of the changes before you can really answer these questions. You might need to follow a few links or talk to the product team. This is exactly why we want human beings focused on these questions and why we push for automation of things that can be automated.

Projects at Spiff are generally setup to automatically deploy when we merge to master. The project README should specify if this is not the case.

The Hippocratic Oath of the Shipit

When you are reviewing code from a teammate, it’s your responsibility to protect and increase the quality of our system. Much like the Hippocratic Oath, we need to “first do no harm”. Specifically you should consider the following types of harm:

If you can’t answer all of the questions above with confidence, consider leaving a comment along the lines of:

Looks good in terms of code quality, but I’m not sure about performance. Have we done any benchmarks against production data?

This is going to be great for the onboarding team❤️. Is it safe for us to change this part of the graphQL? Or should we add a new field so the frontend code can switch to it in a backwards-compatible way?