How Are Your Pull Requests?
Don't make pull requests like pulling teeth.
Join the DZone community and get the full member experience.Join For Free
My college life started in the town of Boston, MA — about 950 miles from my hometown in the Midwest. Since I was living in a large city, paying for parking for my automobile was not an option. My sister and her friend drove me to Boston and helped get me unloaded before headed back to the Midwest. I was grateful.
This was the late 1980s.
The only real way I had to communicate with people back home was to call (long distance phone calls were expensive) or write letters. As you might imagine, I wrote letters ... quite a few of them, if I can be honest.
The response time for a letter mailed from a first-class stamp was far from quick. I mean, the post office could get the letter there in two or three days, but the other person had to not only read the letter, but actually take time to write a letter back to you ... then find time to mail it. I really don't think most college kids today can understand the world as it was when I attended college.
What I quickly learned is that I knew who were good responders and bad responders for my communication back home. If part of my goal was to receive a timely response, I would make sure to focus on those good responders, since they seemed just as interested in communicating back to me.
This story from my past made me think about pull requests (PR) in our daily programming efforts.
Below, are five items we can apply to PRs as a result of my college-days example.
You may also like: Gitting Started With Git.
1. Be Responsive
In organizations where multiple reviewers are attached to a PR but only one reviewer is required before merging, it is easy to skip the PR and focus on something else.
However, in doing so, you are truly missing out on the opportunity to be a part of the candidate logic being accepted into the primary code stream. In fact, your knowledge of the area of interest under review may provide insight not immediately known to others.
In my college days, I recognized the value of responding to letters — even when I found myself at the receiving end of a long-distance communication later in life.
2. Constructive Criticism Is Key
Being professional and polite is a very important trait of a reviewer in the PR cycle. Criticism should always be constructive to help the submitter see your point of view without feeling the comment is some type of personal attack.
The entire college experience was filled with constructive criticism for me. Looking back, I am glad those who crossed my path took this approach — especially during those "what was I thinking?" moments where my idea was really not-so-great.
3. It's Not Your Way or the Highway
As the reviewer of a PR, taking an approach that demands your suggestions are implemented is not a favorable decision. Otherwise, your comments become their own type of PR themselves, which are not reviewed by others.
In other words, if the provided comments introduces an alternate approach that doesn't really fit the aspect under review, asking the submitter to make your changes and merge them is likely worse than using the original code in place. This is especially true if you are the lone reviewer on the PR.
In my limited communication scenario, this is similar to making demands to "write back, asap" at the end of my letters. Most likely, those words have zero impact on whether or not I would receive a return response.
4. An Opportunity to Learn
Building upon item number three, we always have to keep in mind that the PR should be viewed as an opportunity to learn. This is not limited to the submitter, but to the reviewer as well.
During my current role, I find myself performing code reviews and PRs across multiple clients on a daily basis. The amount of times I learn something from a peer review of program code is higher than I would have ever expected before I was put into this position.
I try my best to supplement larger comments, using reference links when possible to provide not only an answer but some background on why my suggestions are being made.
With all communication, it is important to communication something of value to the receiver. Doing so not only provides value for the time taken, but also provides a learning opportunity.
5. Focus on Delivering a Quality Product
By far, the biggest value I always try to keep in mind with the PR process is deliver a quality product to the customer.
This becomes more important when put into the role of being the final reviewer in a series of PRs for a client. With limited resources available, those PRs will not be seen by employees at the client. Instead, there is a trust instilled that I will be acting in the best interest of the client.
While I could take the opportunity to rubber-stamp the PR and assume the prior reviews considered the project's suggested standards, I would not be focused on delivering a quality product. In this particular case, one only has to reference the legacy solution at the client to see the negative impact from not using the PR process to help the quality of the logic entering the code stream.
In this instance, if the letters I was writing did not provide any value to the reader, not only will they stop reading my correspondence, but my chances of a return letter are quite dismal.
For the price of a postage stamp and a great deal of my personal time, I learned an important lesson while attending a college hundreds of miles from my hometown. In an age when long-distance communication options were limited, I took a second to understand my goals before writing a letter to a friend or family member back home.
This same ideal can be applied to pull requests. While part of our responsibility as an IT professional, making the most of a pull request will not only lead to a better code base, but provide a learning opportunity (for both the reviewer and the reviewee) along the way.
Have a really great day!
Opinions expressed by DZone contributors are their own.