InRhythm

Your partners in accelerated digital transformation

Programming

How to Write a Good Pull Request

Pull Requests are the backbone of open source software development. They allow contributions by anyone, from anywhere. PRs are also a vital form of communication, even within a localized development team working on proprietary software. What makes a good Pull Request? Let’s break it down into 4 rules of thumb:

1. Provide context

2. Make it as small as possible, but not any smaller

3. Take screenshots

4. Ask for assistance

Provide context

Providing context is an important first step in guiding the reviewer. Use this as an opportunity to explain why you are making this change. This can be as simple as referring to the bug/defect/issue number, and as detailed as necessary to describe your change.

In this example, I provided a link to the original issue on the npm.community site and linked directly to the source of pnpm that was referenced in the original issue.

https://github.com/npm/cli/pull/32#issue-204418739

In this more complicated example, I wanted to make sure I was consistent with how other PRs were made to this repository. One of my suggestions is the old when in Rome rule. Some repositories even provide a template, which can be helpful, but this one didn’t. So, I made each item discussed in the issue into a bullet item, checked off the ones I completed and noted anything I wasn’t sure about.

https://github.com/npm/cli/pull/61#issue-211591081

Make it as small as possible, but not any smaller

Everyone agrees that smaller PRs are easier to review. Sometimes it’s just not possible to make a very small change, so here’s some practical advice: don’t do more than necessary. If you need to stray from the core of the change you are making, separate it. Here’s an example – imagine you are adding a path through the code to handle a new requirement. Along the way you realize that some of the variables and functions could use better names, but changing those means that you now need to update a bunch of files in another area of the code. STOP! Don’t make that change! Or at least, don’t bunch it together with your other changes. Instead, make the rename change in its own commit, and probably in its own Pull Request too. In the PR you can explain why you are making the change and how it simplifies the real change you want to make easier.

This same strategy should be applied to most whitespace and refactorings you want to do during the course of implementing a new feature or resolving a defect. Be considerate of the reviewer’s time. There is nothing more frustrating than hunting through all the changes looking for the actual change and bumping into whitespace and renamings spread across many files.

Take screenshots

So you’re working on a story that affects the UI? Maybe you are fixing alignment in IE11, or adding a new interstitial modal when the user clicks a button. The code will get reviewed as it always does, but many people (like me) struggle with visualizing layout changes or CSS tweaks. Include a screen capture of the before & after. It’s usually pretty easy to get a before shot – just use the QA or production environment. Then for the after shot, use your local server. Both GitHub and Atlassian BitBucket allow you to paste images, so you can literally SHIFT-CTRL-CMD-4 (OSX) to copy a section of the screen to your clipboard, then CMD-V to paste it into the input box of your PR description.

Another incredibly helpful option is to use an application like GIPHY Capture to record an animated GIF that can be added to your PR. These are great for when you want to show an animation or a sequence of steps. Let’s face it, it’s a pain for the reviewers to fire up their Windows vm to try out your change that resolves an Edge problem. Make their life easier by including an animated GIF that shows exactly what changed right in the Pull Request!

Ask for assistance

I am a big fan of the quote “it is better to beg for forgiveness rather than ask for permission”. In so many instances, not just in software, this rule helps save time. But making too big a change in your PR may be received poorly, especially when you are not a regular contributing member. I’m not advocating “playing dumb,” but being cautious and curious can help lead to better engagement and ultimately a better solution.

Here is a comment I made on a PR to the tslint project I made a few years ago. You can see how I acknowledge the person’s comment/feedback and ask a clarifying question because of the impact it would have on so many files. This lets the reviewers know that I have respect and consideration for the size changes coming into their code base, and also that I want to be collaborative in finding the best solution.

https://github.com/palantir/tslint/pull/1738#issuecomment-261527450

Conclusion

I am hopeful that these points help you make Pull Requests that are more quickly reviewed and accepted. What other things do you like to do in your PRs? What kinds of things would you like to see more of in PRs that you are reviewing? What would you like to see less of? Leave a comment below or hit us up @GetInRhythm on Twitter.

Featured Tech Talk – Andy Philbert Jr. – Javascript Recruiter Extraordinaire

 

Listen to Andy Philbert Jr. – Head Javascript Recruiter of InRhythm, deliver an insightful talk on what his kind look for when interviewing and hiring.

From Andy:
“Recruiters are usually the first people you speak to when interviewing with companies. We are not engineers and screen for non technical qualities that shows that you would be someone amazing to work with and have the potential to be or already are a great engineer. This talk discusses the recruiter’s role within the interview process and how to market yourself as a great engineer without having to rely on just showing your code.”

Session: IR Lightning Talks 1 – Aug 24, 2017, IR HQ

 

When to Pair Program

Follow Jack on twitter @JacopoTarantino for other related topics.

Lots of companies embrace pair programming as a way to increase programmer productivity (loosely defined here as delivering “value” which can have many forms, but is generally interpreted as writing more code per day) but is it really that great? I’ve wondered why we should pair program and when is the right time to embrace it as a strategy.

Pair programming, as I know it, came into popular culture as a facet of XP (extreme programming), a development framework that enforces practices that generally improve software quality and responsiveness. The idea is a new incarnation of the old adage “two minds are better than one”. Either way, the idea is right: two people have different histories, cultures, and experiences, and for those reasons they think about things in different ways. When two people work on a problem together they almost always come out with a better solution that if one worked on it alone. So how does this relate to programming?

The Good

Pair programming does reduce bugs

The primary driver for pair programming is to increase quality and decrease bugs. When done well, it does that spectacularly. One study found that pairing reduced bugs in production by 15%!

Pairing does increase code quality

Many of the benefits of pair programming are not actually technical; they’re social. When working with a peer, it’s normal to feel encouraged to do your best, ensuring your coding is clean and avoiding any technical debt that you’ll “fix later”.  When pairing, we tend to do things just a little bit cleaner, make algorithms just a little bit easier to read, name variables a little bit more sensibly. We actually write our unit tests to 100% coverage! With two sets of eyes, the quality is always higher.

The Gotchas

Pair programming does not eliminate bugs.

As much as we’d love for pairing to just eliminate all bugs, it’s just not the reality. Bugs still happen. There are generally a lot fewer of them, but perfect code would require perfect programmers.

Pair programming does not fix bad product direction.

Good projects need very strong product direction. And to be clear, the responsibility for this direction is on everyone, not just “product people”. It begins with asking questions and making informed decisions about work to be done. Then the team needs to thoroughly discuss the work, breaking it down as much as possible to understand the full scope. If this isn’t done properly, deadlines are missed, everyone is stressed, work that should take minutes takes days… Pair programming can’t fix that. Nothing is more important than good product direction.

Pair programming costs a lot.

Like, a lot. One study found that pairing increases the cost of software by 100%. While that seems like a predictable number (1+1=2), it’s actually highly variable. If pairing is used effectively, I would say it strictly costs the amount of time put in, minus the value added. It could be a single-digit percentage. On the other hand, if pairing is done poorly, you’re paying about 300% more. That’s 100% for putting another salaried employee on the same project, 100% for that that employee not adding any value, and 100% for the opportunity cost of other work they could have been doing. Make sure you’re doing it for the right reasons and utilizing the right resources.

You have to do pair programming right for it to mean anything.

Pair programming is a tool meant to help make a difficult problem more digestible. Pair programming done correctly generally means one person is writing code and the other is directing the work. Directing in this case means providing feedback about best practices and constructive criticism. It also means researching those best practices when you don’t know them off the top of your head and taking the time to think deeply about possible edge cases and hangups relevant to the work at hand. Pairs should communicate thoroughly, share all relevant information about their work, and swap duties as often as possible. It’s taxing to think about problems in both a creative and technical way, so it’s better to distribute that work. That’s one big reason that pair programming is such an effective tool.

So when are we supposed to pair program?

Pair program when there is a *very* difficult problem at hand.

If you have a problem you cannot reasonably break a big problem down into smaller parts,  that’s the kind of thing that should be attacked by multiple programmers. An 8-point story should generally never exist in your organization if you’re doing normal web work. Features can almost always be broken down into “front-end” and “back-end” stories. Whole-page mockups can be broken down into component parts. Design and QA phases can also be separated out into their own stories. But a really tough problem is just a really tough problem. Trying to add a new feature to a language is a really tough problem. Trying to figure out how to reduce the latency on database calls is a really tough problem. These are examples of problems that require both creative and technical thinking.

Pair program when 2 programmers are at completely different skill levels.

Pair programming is a remarkably good way to teach junior programmers. Getting to participate live while a more senior programmer talks about how and why they’re doing something is invaluable experience. So is writing code while a more senior programmer coerces better practices on the fly. In my time as a programmer, pairing with somebody significantly more senior than me has been some of the most difficult and most rewarding work I’ve ever done.

Pair program when 2 programmers have completely different skill sets.

If you choose not to break down a story into easily divisible parts, having 2 programmers with complementary skill sets can be very rewarding for both the programmers and the codebase. Pairing programmers who generally only work front-end or back-end can get an end-to-end feature out the door, or you might have a Postgres expert pairing with a Scala expert to make your database calls more efficient. When these 2 people work together live, they absorb a lot of knowledge about each other’s domain and ensure there’s no aspect of the project that’s neglected.

Pair program when you’re both new to a language or framework.

Sometimes you wind up in a situation where nobody is an expert. Even if you’re working on a trivial feature and just wandered into a very difficult problem (see above). This is an excellent opportunity! You end up with two programmers working through a difficult problem and contributing their individual skills to build a better product, helping each other learn, and with a redundancy in the skill growth of your programmers. This is important because the skills in your organization should never be concentrated in one person. Having programmers pair on new languages and/or frameworks ensures that there are a minimum of two people who can work on this in the future.

This post is another one by InRhythm’s own Jack Tarantino. For the full post, check out the original “when to pair program” on his website.

April Meetup Video – Building Light Deployment Free JSON API’s with Dropwizard

 

This month we took a walk through Dropwizard city with out very own Luke Lappin. He is a senior enterprise architect with us who also happens to be an incredible guitar player.

As always, we have full video of the entire talk for you. We hope to provide you with a great step into Dropwizard.

Luke starts off by giving a quick background into Java and explaining that Dropwizard was created to be the glue between all these established Java packages. With this framework you can easily create REST and JSON services in the fastest way possible. Cool stuff!

Enterprise Java can be frustrating but it doesn’t need to be. Luke takes us through all the meat and potatoes of Dropwizard ingredient by ingredient. Tell us what you think in the comments.

Missed out on our April Meetup? Make sure to join our Meetup group and never miss out again! Follow us on Twitter for exclusive Meetup updates. We announce all topics and speakers on there first! Hope to see you next month.