[27] 3 Components of Reviewing a Pull Request (scikit-learn) (Thomas Fan)

## Upcoming Events
Join our Meetup group for more events!

# Thomas Fan: 3 Components of Reviewing a Pull Request

## Key Links

## Resources

## Agenda
00:00:00 Reshama introduces Data Umbrella
00:05:20 Thomas begins talk
00:06:35 Terms: pull request, reviewer, contributor, merged
00:07:12 PART 1: Mechanics of Code Review. Why code review?
00:19:10 Browsing Code in GitHub
00:21:50 shift+click: highlight multiple lines
00:22:18 GitHub b: blame, tells last contributor to the code
00:23:12 Overview of Mechanics
00:24:12 Q: How does a suggestion get incorporated into the code? how to sync local repo?
00:27:02 PART 2: Social Aspects of Reviewing a Pull Request
00:27:58 Tip 1 for giving feedback: Ask, don't tell
00:28:46 Tip 2 for giving feedback: Offer alternative implementations
00:29:56 Tip 3 for giving feedback: Explain why code should be changed
00:39:56 DRY: Don't Repeat Yourself; refactor, having common logic in multiple places, rewrite code so you have same funtionality, but this makes code more maintainable
00:31:50 Avoid using derogatory terms
00:32:32 Tip 4: Be humble, assume everyone is intelligent and well-meaning
00:33:15 Tip 5: Ask for clarification
00:34:11 Tip 6: Communicate which ideas you do not feel strongly about
00:35:02 Tip 7: Try to leave a positive comment
00:35:33 Tip 8: Thank contributors for their work
00:35:56 General Social Aspects
00:36:37 Disagreement
00:37:39 Talk synchronously (talk, take notes, update PR with notes to document discussion)
00:38:45 Overview of Social Aspects
00:39:05 PART 3: Technical Aspects
00:39:52 Presentation: how your PR is presented? description of PR
00:42:00 Was unrelated code changed?
00:42:55 Is the PR easy to review?
00:44:45 Engineering, Technical Aspects
00:44:58 Every change should be intentional
00:45:10 Offer ways to simplify or improve code
00:46:20 Does the change maintain backwards compatibility
00:48:42 Performance optimizations, require benchmarks
00:49:32 Is there documentation?
00:51:05 Code comments
00:52:15 Testing! (are there tests, and more)
00:54:08 What does it mean when CI tests fail?
00:55:42 It's ok to make mistakes
00:56:53 Q&A

## Event
A pull request is a method for submitting contributions to a software project. Maintainers or contributors review these pull requests to discuss the proposed changes and help ensure the changes meet the project guidelines and quality standards. In this talk, we will learn about three components for reviewing a pull request:

1. The mechanics of code review on GitHub.
2. The social aspects of code review and how to effectively give feedback.
3. The technical aspects of reviewing a pull request.

## About the Speaker
Thomas J. Fan is a maintainer for scikit-learn, an open-source machine learning library for Python. He leads the scikit-learn project by designing the project’s roadmap, giving feedback to contributors, and implementing new features. Fan also is a maintainer for skorch, a scikit-learn compatible library that wraps PyTorch. He has a Masters in Mathematics from NYU and a Masters in Physics from Stony Brook University. During his academic studies, Fan researched quantum computation and condensed matter physics.

#opensource #etiquette #github