If you are a PR writer

1. The PR should not be too large

Keeping Pull requests (PR) to a minimum is an art form. While writing code, you often feel the urge to rewrite, refactor, or reformat your code, but in general, good developers resist the temptation to change everything at once. They focus on one goal and minimize the amount of code that needs to be changed. Some people even compare “lines of code deleted” to “lines of code added”. If you need to refactor and optimize your code, do it separately. Don’t make excuses to cram all the changes into one PR, it’s lazy.

Instead, find a way to keep your PR low. That’s creativity.

2. Self-censorship

Once you’ve created your PR, you should do a thorough self-review. After completing a piece of code, you might be tempted to push it to PR and wait for others to catch the bug, especially if it took a few days to complete a major change. Don’t be lazy. Be self-disciplined. Your work isn’t done.

In your self-censorship, ask yourself, “Is this a good name? Is there a better one?” Or, “Can this really be NULL?” By asking questions like these, you can not only check your own code, but also establish a good habit of self-reflection in your daily programming work. In other words, this process of self-censorship can make you a better developer.

3. Remove unnecessary files

In your self-review, you’ll often find a file with a few whitespace changes, formatting tweaks, import optimizations, or text changes that have no impact on PR at all.

In this case, you should open the branch again and restore the files back. You have only made some minor improvements, but the functionality is unchanged, and the presence of redundant files in the PR will only burden the person who reviewed the code, especially if the PR is large.

If formatting is important, create a separate PR, then add a formatting tool to the CI, and use the tool to format the entire application. However, it is important to avoid these unnecessary files out of respect for code reviewers. In addition, these insignificant changes can contaminate Git’s history, making it difficult to trace the intent behind certain changes to the file.

4. Create meaningful headings

In general, we will copy the branch name or the related note for the title. There is only one rule for headings, which is to follow a convention to create concise and meaningful headings, with the same basic idea as branch naming.

Creating pull requests is also creating documentation, keeping a history of pull requests easy to browse and making it easy to search past decisions and thought processes.

5. Meaningful descriptions

Again, you should think of PR as a document, a document that is often read. The description of PR should be as comprehensive as possible, but be as concise as possible to understand your intention and decision-making process through the description without spending time discussing it.

One effective way to do this is to create a PR template. The content of the template should be agreed with the team and developed and adjusted over time. Here are some tips for novices:

  • Change Overview. You need to explain options that were not included in the PR (alternatives that were evaluated and abandoned). This will avoid having to rehash what you’ve already tried with the person reviewing the code.

  • Questions/comments for reviewers. What specific code do you want reviewers to suggest? What parts of the code are safe to ignore? What part of the code did you refactor? There were a lot of files that changed, but there was no functional impact. Tell the reviewer what part of PR they can safely skip, and they’ll thank you very much.

  • How to test/demonstrate. It is very convenient for QA to have instructions such as users and passwords in the pre-release environment, configuration and setup instructions, etc., anything that can help reviewers test changes.

  • Attachment: screen shots and videos. Pictures and videos are more expressive. Pre – and post-change demonstrations are handy.

6. Confirm each comment

In asynchronous remote communication, it is important to communicate that you have seen the comment. Sometimes, all it takes is a simple emoji. No matter how small a comment is, it can’t be ignored, especially on a new team. Once you build rapport with your team, you can let it go because you all know each other. However, at the beginning, it is important to be polite, mindful of your words and personal behavior.

7. Mergers require everyone’s consent

Speaking of politeness, you should wait for someone to give you an opinion and then respond positively. If the wait is too long, you can ask by email and instant message, or just go to them and let them know you’re still waiting.

In my opinion, if you have more than 3 people on your team, you don’t have to have everyone vetting every PR. Develop a system to determine who will review each PR, such as making each person responsible for some modules; If you are new, have the architect/senior developer review all of your PR.

Two, if you are a censor

Some of the suggestions below are also applicable to code writers responding to comments.

8. Check out code

You should always have two clones of a project on your computer: one for normal work and one for reviewing PR. This way, when reviewing PR, it doesn’t interfere with the current task.

After checking out the branch, click Build and leave it running while switching to the browser.

9. Read the title and instructions

If someone has gone to a lot of trouble writing their own PR guide, you should at least be patient and read through it. While waiting for the PR to be built on another clone of your project, please read the relevant ticket, read the PR title and instructions, and give your comments for review.

10. Validate your suggestions locally

If you see something that can be improved, try changing the code in a local clone. This is especially important when you are new to a project. Don’t be embarrassed if your suggestion doesn’t work, or even doesn’t compile at all. Modify code in your OWN IDE, and if you succeed, you’ll get a great sense of accomplishment. If you fail, you’ll be glad you didn’t disturb anyone.

After confirming that your proposal works, don’t let your work go to waste by copying the code and putting it in a PR comment so the author can copy it directly.

11. If the suggestion is too big, just write PR

If you find that your proposal is too big, don’t waste energy, just build a branch on their branch, and create a PR to merge into the original PR. This kind of PR doesn’t need the bells and whistles of regular PR, because it’s just part of a comment that you can explore further, then be considered for modification by the original author, and finally, if all goes well, incorporated into the main PR.

Resist the urge to comment

When making comments, we can’t help but gush. The key to fighting this emotion is to put yourself in the other person’s shoes. Don’t forget to review all of your comments and think carefully about how many of them will actually be accepted and can be done as quickly as possible.

Here are some tips for commenting:

13. Don’t comment without a better alternative

If you don’t like the way the code is written, come up with a better way or shut up.

14. Be confident and don’t be lazy

Don’t use words like “maybe,” “I don’t know,” “if,” or “I’m not sure” that suggest doubt. If you’re not sure, ask yourself why you’re not sure. Or you can do some experiments and research to regain your confidence.

15. Copy the original style before changing the code

Everyone has their own style, and they are more attached to their own style. When joining a new team, existing members will often defend the status quo, while new members will express opinions about how their previous project was different or how they could have done their job better.

Adapting to an existing style will get you on board quickly, and they will be more willing to advise on important aspects, such as SDK selection, architectural decisions, patterns and practices. It’s easier for them to accept a modern style after you’ve completely mastered their style.

It’s good to be picky

It can also be frustrating when colleagues review your code and make stylistic comments that seem critical.

However, think of it this way: reviewers are having a hard time finding actual problems in your code.

When faced with critical comments, such as comments about style, you can politely emphasize that IDE tools handle 90% of style issues automatically.

17. Face-to-face communication

Sometimes the two of you have a back-and-forth argument about a PR comment. At this point, you should calm down and write an email to schedule a face-to-face meeting.

Online communication sometimes goes back and forth and can be a waste of time. Go to a conference room and meet face to face, but be calm, mull over your ideas, and be objective. The purpose of face-to-face communication is to find the best solution.

18. Be calm

To establish PR, it is inevitable to be pointed out by reviewers. So, first and foremost: nitpicking. But you need to be professional, not personal.

Pay attention to the way you interpret comments, some people don’t mean it, they just aren’t thinking too much, are in a hurry, or don’t express themselves well enough. Their suggestions were made in good faith.

19. No urgent PR

PR is popular for two reasons:

  1. Asynchronous communication. Developers can review and respond at any time to avoid interruptions or interruptions to their work.

  2. Quality assurance. Before code enters the target branch, it is checked and tested to ensure that the target branch remains clean. Find potential problems in everyday code through teammates.

If you have to merge branches in 10 minutes (which is rarely recommended), don’t send a message asking someone to review the code immediately, just merge. Don’t bother people about process. If you have to merge branches in a short period of time, find someone to pair program, or just merge the PR.

Third, summary

PR is a good habit to get into. Most of the projects I’ve worked on over the past decade have followed this standard approach, and I’m still learning new things and processes about PR in new projects.

The advice above may not be appropriate for every project, and building a team is more important than having strict rules and processes. In a team, we should be friendly to others, but also have confidence and discipline, and lead by example and be strict with ourselves.