The meaningful and well-formatted Pull Request description is a project document that is as valuable as a properly described task.
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 has focused 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:
1. Why is the description for Pull Request important and useful?
2. What should a useful description of Pull Request contain?
3. 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:
https://github.com/b-mueller/sabre/pull/93
https://github.com/b-mueller/sabre/pull/89
https://github.com/b-mueller/sabre/pull/71
Further reading
Here are some additional articles, that are related and useful:
https://hackernoon.com/no-description-provided-8d9e0f3a3abb
https://blog.carbonfive.com/2020/01/06/why-write-good-pull-requests/
https://www.pullrequest.com/blog/writing-a-great-pull-request-description/
Previously published at blog.maddevs.io
Top comments (0)