Good_pull_requests

In this write-up, I intend capture notes about writing / sending authoring a good pull request. I’ve attempted to capture all the bits of best practices, I’ve gathered from my interactions with my colleagues, working in projects that followed good practices. But the most of what I consider good practices today comes from the book I read 3 years ago (Software Engineering at Google). I’ve found there is a concise version of it captured in eng-practices. You can stop reading and jump to the google’s engineering practices page right away at this point and return if are still interested in what I have to say.

A philosophical view I have about most things in the field of software engineering is that it’s far easier to achieve better results just by eliminating some of the bad practices than to force-fit random best practices from all over in an uncontrolled fashion. So, baseline set of practices a team can follow that eliminates bad practices is good enough, to start with.

Baseline with no bad practices.

• Define coding guidelines.
• Enforce defined coding guidelines.
• Have repeatable build system.
• Have repeatable automated tests.
• Use automated builds and automated tests as merge criteria.
• Have documentation to help new team-mates to refer to.

The above and similar basic architectural and tech-infrastructure related decisions have to be taken and conquered as a baseline right at the onset of the project. I tend to use the term Productization for such activities. Therefore, I would refrain from shipping out a piece of software or a software product that hasn’t covered the basics.

Assuming that a software development team has covered the baseline, we can continue detailing some of the best practices that we could potentially implement to achieve good software quality, readability, maintainability etc … Herein, we look at what an author of a pull request can do.

A good pull request.

A good pull request facilitates:

• Traceability
• Readability
• comprehensibility
• Efficient use of reviewers time
• Higher confidence in taking the changes in
• Ultimately a good quality software

Authoring a good pull request.

The following structure could be followed in the direction of authoring better pull requests.

• First Line
• Body
• Small changes
• Test information

First Line

A concise summary of the change written in imperative mood, followed by an empty line.

Body

A multi-line description of the change, detailing the following possible items as the author sees fit in the context of the change.

• The feature ID / Bug ID that this change addresses.
• A brief description problem being solved.
• Historical / Legacy aspects of the software component that’s being modified.
• Improvements or shortcomings that this change addresses.
• Possible design options.
• Why a particular design option was chosen.
• Links to design artifacts.
• Benchmarking / profiling results if applicable.

Essentially, enabling the reviewer to be self-sufficient to review the code changes with the required amounth of confidence. As it happens, when coding, empathy is a virtue, just as naming variables and functions and data structures are important, a non verbal, digitalised, mostly asynchronous collaboration with a colleague requires equal degree of empathy on behalf of the author. One of the reasons why everyone on the team must always review code changes.

Small changes

In the creative field of software development (Yes, authoring a piece of software is a creative process, folks who feel otherwise likely chose wrong field to work in), there is a inherent complexity of working with other humans and there is absolutely no knowing the workload, available time, mood of the reviewers to expect them to very thorough with it. Authoring small changes helps in simplifying this complexity to some extent, enabling teams move forward efficiently. Quite a simple approach to making small changes is to think of the software in layers, identifying the changes that does one particular thing, make code changes, write test code and author the pull request.

Test information

Code changes must always accompany related test information, when automated tests are in place, a new of modified test case should be part of the same pull request. Otherwise, test results as part of the pull request description.