Before you go, check out these stories!

0
Hackernoon logoHow to Create A Detailed Description for Your Pull Request by@gen1us2k

How to Create A Detailed Description for Your Pull Request

Author profile picture

@gen1us2kAndrew

CTO at Mad Devs

Many developers are familiar with the situation like “where did this code fragment come from and why is it needed?”. You have to spend time and deal with the details already considered by another colleague. How to make it take a less amount of time? To achieve this, pay attention to a process of writing descriptions for Pull Requests (known as “PRs”) and Merge Requests (known as “MRs”). This article will focus on the content of the PR description without any explanation of coding since each project has its own coding-related specifications and requirements.

Here is the list of topics that are covered in the article:

  • Why is the description for Pull Request important and useful?
  • What should a useful description of Pull Request contain?
  • Which action helps writing a proper Pull Request description?

Why the description for Pull Request is important and useful

First of all, let’s try to understand why it is better to pay attention to the description for PR.

Working on a project is a long path that’s divided into short segments. The beginning of each segment is a task, the steps are commits, and the end is PR.

The importance of PR is rising when you start to realize that PR is the result of your work on the task. Superficially, it seems that the most important part is the commits with code that introduces functionality, optimizes the work of the final product.

However, there is also another thing — the context of the changes that you apply to the product and the reasoning. 

The context implies your rationale for the chosen implementation method, the selected tools, the encountered issues, and the dilemmas.

Providing context for PR is useful since the task itself and its discussion might be not sufficient enough to understand the full picture of the changes. It is good, when the discussion was conducted in the issue tracker or, verbally or in the chat, and was written in the form of short results.

If not, then you will have to answer the questions like 

“where does this code come from and why is it needed here?”

As a reviewer, I don’t feel joy during a PR review if I have to spend extra time to understand the logic of code and commits, because the author did not bother to write a proper description. When the coding part is done, the author of the changes could dedicate some time to the general description of PR, while everything is still fresh in memory. In this way, the author would save the time and effort of the project team. Let’s list the project members, who can benefit from the properly described PR:

  • Reviewers no longer have to guess what you wanted to achieve with the changes. There is no longer a need to spend time asking questions, like “Why did you pick this implementation path and not another?” or “Why did you choose the particular approach or selected some dependency?
  • Project colleagues while looking at your commit can find a PR with a description of the changes made and a link to the task with related discussions. In the end, this will help to avoid bugs, since if your code needs to be altered in some way, colleagues can find out the requirements for its logic and pay attention to the relevant details.
  • You won’t need to answer additional questions about the implemented changes. In such cases, you can always refer to the description, where the most important points have already been clarified. This becomes even more relevant over time — since after like six months, it will be difficult to remember why the particular change was made. Anyway, there is no need to do that if the changes are backed up by a meaningful description.

This all leads to a conclusion:

The description of PR is as valuable as the task ticket.

Presenting the results of the work in the description for PR is important and useful, as it will save precious time and energy for the future.

What should a useful description of Pull Request contain

Now let’s consider important parts to cover to write a useful description for PR.

Title

Start with the title — it should reflect the essence of the changes. If there is a corresponding task or ticket, then the PR title can overlap with it. In such a case, a prefix with the task number should be added.

SLI-001, SLI-002 - Support for reading input from STDIN
SLI-002 - Bugfix for memory leak when input is supplied via STDIN

The title should preferably be informative and short enough.

Related tasks

Tasks and tickets that are related to or will be solved by the PR must be provided. Better to provide them as a list with links to tickets in the tracker. In MarkDown, it looks the following way:

## Related tasks
- [SLI-001](http://my.issue.tracker/project/SLI-001)
- [SLI-002](http://my.issue.tracker/project/SLI-002)

Such an approach greatly simplifies navigation. For tasks that are located on GitHub and GitLab, there are special phrases that can be used in PR to close the referenced issues (manual for GH and manual for GL).

Dependence on other PR

If the PR is based on other open PRs that should be reviewed first, then they should be listed. This also applies to PRs for used packages and dependencies. Here is the example in MarkDown:

## Depends on
- #3             (other opened PR in same repository)
- dependency#55  (other PR in the other related repository)

This will optimize the PR review process and save time for reviewers. If your PR does not depend on anything, skip the section.

Premise

This section is dedicated to preliminary research or other things that were preceding the applied changes. The following should be covered here:

If you made a choice between possible packages and chose the most suitable one, then you should provide links to each package, then describe its “pros and cons” for solving the particular problem. You should also state the final reason why you have chosen a specific package for the integration into the project.

If there was research about a task-related topic or an investigation for a different implementation approach, then you should provide links to the articles that were helpful in understanding the solution. You can also describe key points that will help to understand the changes that will follow.

If you discovered that the existing code is disturbing you, then it should be outlined. If you were forced to refactor some function due to it being poorly implemented or affecting your implementation in some way, then it would be better to explain why such change was considered as necessary.

Brief results of discussions with the team, which will help to understand why certain decisions were made.

An example may look like following:

## Premise
As we have grammar description and access to an 
assembly code even from AST, I considered to use 
PEG-based parser to generate custom AST from 
assembly code, then use adapter to make it 
compatible with current ASTReader processing logic. 
As prerequisites I've looked through the following 
packages:

- https://github.com/navstev0/nodebnf (npm) - I 
tried and in the end came to the conclusion, that 
it is not so stable to be used for our needs. Also, 
lacks proper documentation.

- https://github.com/lys-lang/node-ebnf (npm) - Has 
proper documentation and TypeScript definitions. 
Also, has a useful online sandbox. Chosen for the 
further implementation.

This section is used for general cases when it is necessary to present any information in a reasonable volume that the reviewer or any other colleague should know before they begin the inspection of the changes of PR. If there is no such need, then the section should be skipped.

Changes

The section should list the changes that are applied. If you previously described the details in the messages of commits, then you can reuse the text from there.

If you think that some points require additional explanation, then you can indicate important information as sub-items.

## Changes
- Added dependency [ebnf](https://www.npmjs.com/package/ebnf).
- Cleaned **package.json** from some obsolete 
commands. Introduced `ebnf:update` to copy BNF-
files when post-install occurs.
- Introduced **src/ebnf** directory with components 
to parse code strings by BNF definitions.
- Tweaked configuration to be able to work with a 
new set of components.
- Introduced tests to cover new functionality.

Here are some guidelines for formulating the contents for the section:

Some changes are available via reviewing the diff. Therefore, do not micro-describe — GIT will do it for you. It is better to summarize the changes, stating their meaning. For instance:

- The dependency versions are updated because they 
are very outdated and the security audit detected 
vulnerabilities.

- Files in the X directory were deleted because the 
component that worked with them was replaced with 
another. They were no longer needed.

- The method was updated because the low-level API 
has changed, and because of this, the method no 
longer worked properly.

When justifying actions related to dependencies, do not hesitate to use links to releases, merged bug fixes or features — it is going to be very useful in the future.

If you introduce a new class, function, method, or other routines, it is better to describe its purpose (or even better, do it in the doc block in front of its definition).

Try to pay a bit more attention to the actions that cannot be covered by comments in files with the source code: moving files, deletions, automatic updates (dependency configuration, for example).

Some additional features, that you also should be aware about:

  • You can use the “task list” option that is available on GitHub and GitLab. The list items can thus be made interactive.
  • For clarity reasons, do not hesitate to refer to the code segments that you mean. You can read how to do this here for GitHub (I was unable to find instructions for GitLab and will be glad if somebody will share the link).

Concerns

If you have any doubts or feel contradictory, regarding the implementation, then mention in PR. Feel free to state your concerns and ask for advice from project colleagues. Here is a short list of possible reason to mention:

  • You are recently on a project or not yet experienced in using a particular technology. So far, you did what you could, but you are unsure if the implementation is reliable.
  • You encountered an issue that you don’t know how to solve, and advice from your colleagues would be helpful.
  • You encountered a lack of time or additional difficulties and left part of the functionality unimplemented by placing a “stub” in the code. Ask about suggestions or state that you could use a bit more time to finalize the work.
  • You might suggest another more effective approach to implementation, but you don’t know whether it is worth applying now, or it would be better to postpone the implementation to a later stage of development (depending on the circumstances).

Here is an example in Markdown:

## Concerns
It appears that the compiler is trimming spaces and 
reducing indentation within assembly code strings 
in `Assembly` AST node. This causes inability to 
generate proper `src` attributes for custom AST by 
simple math. Also, this is not fixable, because 
these dependencies are already released and can not 
be patched. Need to find another way to link 
`AssemblyIdentifier` to a top-level 
`VariableDeclaration` node. Currently, any 
`AssemblyItem` node will have the same `src` as 
parent `Assembly` node. Maybe I'm missing 
something... I would appreciate any suggestions 
that will help in solving the situation.

If you have not encountered problems, then you should omit the section.

Notes

This section lists all the aspects that are not obvious or are side effects. For example, you introduced a new build command that copies files, or used some kind of functionality with “magic numbers”.

Better mention such things in advance, because the questions will be asked sooner or later anyway. Here are some examples:

## Notes
- This PR changes major package API, which would 
result in a backward compatibility (BC) break.

- When changes applied to grammar files, developer 
must run `npm run ebnf:update` to copy updated 
grammar files to **dist/**. Otherwise, components 
will not react to any changes. All grammar files 
are static assets, so they are not compiled or 
handled by **tsc** in any way.

- For some reason `process.stdin.fd` is not 
available in type definitions for the current node 
version. It seems that we should upgrade at some 
point. I resolved this via magic 
`fse.readFileSync(0, { encoding: "utf8" })` (as `0` 
should point to `STDIN` for each process) and left 
a comment with the reasoning.

What actions can help to make a proper Pull Request description

A few actions that will allow you to compose the useful PR description.

Please note that GitHub and GitLab provide the ability to create a PR description template (instructions are provided here for GH and here for GL). Discuss with the team if it is worth setting templates in advance.

When writing code, think about how you will compose the commit messages. Well-written commit messages are one of the best sources for the “Changes” section. Do not forget to mention the ticket code from the tracker. Modern trackers (Atlassian Jira, for example) are able to collect data from connected project repositories and generate links to related commits and PR.

Before creating a PR, put yourself in place of a reviewer. Create a PR with only two sections “Tasks” and “Changes” with a “Reserved” mark. Then do a code review by yourself.

This will help to look at the changes from a new angle. Please note that a less experienced colleague may walkthrough with your changes after the merge.

Ask yourself the question: “To not waste time in the future, is it worth giving an additional explanation in the description?”.

Think about what things might raise questions in the long term. If more than two people are involved in the project, then colleagues can start asking questions. If you can answer some of them in advance — this is a good reason to expand the description of PR.

Next, try to put yourself out of context a bit and think about what other aspects that may be important to the reader. The project may need to be transferred to other developers. In this case, it will be necessary to write instructions.

Ask yourself, “Isn’t it better to state important things now, while everything is fresh in my memory?”

Remember if some important implementation decisions were discussed with your colleagues verbally or on other resources. If discussions have taken place, then take a little time to describe the summary. Feel free to provide links to useful articles that you have read and which will help to understand the logic of changes or decisions made.

Consider to make the description more explicit. Pay attention to the possibility of using labels to highlight that your PR contains changes in dependencies, backward incompatibility, bug fixes, or new features. Labels increase the explicitness and help with the search (for example, finding all bug fixes becomes a matter of one click).

GitHub and GitLab allow you to use the entire MarkDown formatting arsenal (instructions are here for GH and here for GL). Proper formatting and structure will help to facilitate readability and simplify the understanding of many things. If the changes were applied to the GUI, then consider to supply screenshots of GUI state “before” and “after” the changes.

Remember that well-described PRs are helping with writing project documentation, in particular the “ChangeLog”, the “Release Notes”, the “Read Me”. For the first two, it is enough to indicate links to PRs, for “Read Me”, there is an ability to paste part of the content.

By composing a meaningful description of PR, you will help yourself and your colleagues not to waste time re-reading the code before the release of the new version — it would be narrowed to looking through the merged PRs since the time of the last release.

Conclusion

With this article, I tried to highlight that a meaningful and well-formatted PR description is a project document that is as valuable as a properly described ticket. If you have written the code for several hours, then it is also worth dedicating half an hour to summarize the work.

A habit to write good PR descriptions will greatly simplify the life for you and the project team, also saving a lot of time during reviews and discussions.

Living examples

Here are some of the PRs for an open-source project that can serve as an example:

Further reading

Here are some additional articles, that are related and useful:

Previously published at https://blog.maddevs.io/how-to-make-a-proper-description-for-a-pull-request-5e1cbe9fb374

Tags

Become a Hackolyte

Level up your reading game by joining Hacker Noon now!