Pull Request, Make & Review

Discussion about Pull Request, Making and Reviewing them

Transcript

1. Pull Request
4. Why Pull Request We want someone else to confirm that our code is acceptable.
5. Make great PR Review PR greatly - Reduce or prevent bug from being shipped - Ship faster
6. Time is the cost Coding time Make PR Time Review PR Time Coding time Make PR Time Review PR Time
7. Before Making & Reviewing PR Automate everything that can be automated to prevent wasteful argument within the team: - Use code formatter like Prettier. - Use linter. - Use generator. For example, prisma automatically generates the TypeScript types. - Use code analyser like Codescene or Sonarcloud.
8. Make PR: Link the ticket So that everyone can know what bug the PR will solve or what feature the PR will add. In the ticket, we can put the detailed information.
9. Make PR: Brief explanation what the PR is about To save reviewer’s time, briefly explain what the PR is about after or before mentioning the ticket. This PR resolves fixing the bug where user tries to login with wrong email or password (issue #123).
10. Make PR: Screenshots and/or videos! For the reviewer: Help to see the end result of the PR which can make it easier to make sense of the code. For the PR Author: Help to confirm that the changes actually does what it’s supposed to do.
11. Make PR: Step by step to test Describe the steps to correctly test/confirm the changes. So that the reviewer can reproduce and confirm the PR.
12. Make PR: Keep it small Keep the changes limited to the scope of the ticket.
13. PR Review See screenshots & videos to confirm that the PR actually does what it says.
14. PR Review Make sure bug fix has tests! So that it won’t happen again in the future.
15. PR Review Check if documentation exists! E.g., when adding new feature, is it documented.
16. PR Review Check function/method’s logic. Does it have tests? Function that has no dependencies to external source and not UI related must have tests. const isEven = (n: number) => n % 2 === 1
17. PR Review Check error handling. - Is there code that is executed when error happens?
18. PR Review Check edge cases. - E.g., a function repeats a piece of code as many times as the number entered by user. What happen if number is too large?
19. PR Review Check common mistakes - E.g., using useEffect when not needed.
20. PR Review: Be polite and helpful - “This code sucks” - Explain how you think it could be better. - Add links to references if needed.