Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Effective Pull Request Reviews

Bas Broek
October 08, 2021

Effective Pull Request Reviews

Pull request (and their associated reviews) are an integral part of a developer’s life. And for good reasons!

While some may love them, and others somewhat less so, there’s a good chance you’re spending quite some time on reviews as part of your project or job.

Therefore, it’s crucial to make our pull request experience great. And kind. And welcoming.

I want to share some thoughts on what I’ve learned about making the pull request experience awesome.

Bas Broek

October 08, 2021
Tweet

More Decks by Bas Broek

Other Decks in Programming

Transcript

  1. Effective Pull Request
    Reviews
    @basthomas 1

    View full-size slide

  2. Bas Broek
    @basthomas 2

    View full-size slide

  3. Bas Broek
    » Accessibility
    @basthomas 2

    View full-size slide

  4. Bas Broek
    » Accessibility
    » Testability
    @basthomas 2

    View full-size slide

  5. Bas Broek
    » Accessibility
    » Testability
    » Communication & Collaboration
    @basthomas 2

    View full-size slide

  6. Bas Broek
    » Accessibility
    » Testability
    » Communication & Collaboration
    » (and Watches)
    @basthomas 2

    View full-size slide

  7. Effective Pull Request
    Reviews
    @basthomas 3

    View full-size slide

  8. Developer Playground
    @basthomas 4

    View full-size slide

  9. (Unique?) Teamwork
    @basthomas 5

    View full-size slide

  10. From "Your" Code to
    "Our" Code
    @basthomas 6

    View full-size slide

  11. Shared Responsibility
    @basthomas 7

    View full-size slide

  12. Effective Pull Request
    Reviews
    @basthomas 8

    View full-size slide

  13. It Goes Both Ways
    @basthomas 9

    View full-size slide

  14. Keep it to the Point
    @basthomas 10

    View full-size slide

  15. Code.
    Tests.
    Documentation.
    @basthomas 11

    View full-size slide

  16. Explain Your Thought
    Process
    @basthomas 12

    View full-size slide

  17. How can this be Tested?
    @basthomas 13

    View full-size slide

  18. Visualise
    @basthomas 14

    View full-size slide

  19. Assume Best Intentions
    @basthomas 15

    View full-size slide

  20. Manage Expectations
    @basthomas 16

    View full-size slide

  21. Soak Up Knowledge
    @basthomas 17

    View full-size slide

  22. Read the Ticket and
    Description
    @basthomas 18

    View full-size slide

  23. Ask Questions
    @basthomas 19

    View full-size slide

  24. ... Ask More Questions
    @basthomas 20

    View full-size slide

  25. Self-Review
    @basthomas 21

    View full-size slide

  26. Re-Read a Review
    @basthomas 22

    View full-size slide

  27. Share Knowledge
    @basthomas 23

    View full-size slide

  28. Provide Context
    @basthomas 24

    View full-size slide

  29. Provide References
    @basthomas 25

    View full-size slide

  30. // Why?
    /// What?
    @basthomas 26

    View full-size slide

  31. /// Create and return an action with the specified title and behavior.
    /// ...
    public convenience init(
    title: String? = nil,
    image: UIImage,
    isChecked: Bool = false,
    style: UIAlertAction.Style,
    handler: ((UIAlertAction) -> Void)? = nil
    ) {
    self.init(title: title, style: style, handler: handler)
    // Set values for specific keys, as there is no explicit API available.
    setValue(image, forKey: imageKey)
    setValue(isChecked, forKey: isCheckedKey)
    }
    @basthomas 27

    View full-size slide

  32. /// Create and return an action with the specified title and behavior.
    /// ...
    public convenience init(
    title: String? = nil,
    image: UIImage,
    isChecked: Bool = false,
    style: UIAlertAction.Style,
    handler: ((UIAlertAction) -> Void)? = nil
    ) {
    self.init(title: title, style: style, handler: handler)
    // Set values for specific keys, as there is no explicit API available.
    setValue(image, forKey: imageKey)
    setValue(isChecked, forKey: isCheckedKey)
    }
    @basthomas 28

    View full-size slide

  33. What Else?
    @basthomas 29

    View full-size slide

  34. Non-Code Pull Request
    Reviews
    @basthomas 30

    View full-size slide

  35. In-Person Review
    @basthomas 31

    View full-size slide

  36. In-Person Review
    Review Over a Video Call
    @basthomas 32

    View full-size slide

  37. Treat it as if it Were
    Open Source!
    @basthomas 33

    View full-size slide

  38. Git is Forever
    @basthomas 34

    View full-size slide

  39. Your Pull Request Environment May
    Not Be
    @basthomas 35

    View full-size slide

  40. Avoid Derogatory Words
    @basthomas 36

    View full-size slide

  41. The most important thing: Interesting
    work with good people
    — Aaron Hillegass
    @basthomas 37

    View full-size slide

  42. Point Out the Good
    Things
    @basthomas 38

    View full-size slide

  43. Thank you!
    @basthomas
    @basthomas 39

    View full-size slide