- Transcript: https://github.com/data-umbrella/event-transcripts/blob/main/2021/27-thomas-pr.md
- Meetup Event: https://www.meetup.com/data-umbrella/events/278045166/
- Video: https://youtu.be/dyxS9KKCNzA
- GitHub repo: N/A
- Transcriber: ? [needs a transcriber]
https://blog.scikit-learn.org/community/pull-request/
## 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 functionality, 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
