S oftware C raftmanship Z rich How to not suck at Code Reviews ;-) - PowerPoint PPT Presentation

S oftware C raftmanship Z ürich How to not suck at Code Reviews ;-)

Survey time! Code Quality - Happy? Code Reviews @ Job? Commit Review? Pull Review? vs. Code Review? Who is reviewing their own code?

Agenda aka “The plan” 1. Exercise 2. World cafe 3. Tips from videos / blogs a. 3 “code review” smells 4. Closeup & Action

Exercise Groups of 3 2 reviewers, 1 observer Grab some code 15 minutes Timer starts soon...

Exercise: How was it? 5min Feedback in your group 2 min: Come up with 3 points to share to all

Exercise: How was it? Share in the whole group → Only share if something wasn’t said before

World Cafe 7 rounds x ~4min

Code Review Tips From the internetz from RailsConf 2015 - Implementing a Strong Code-Review Culture https://www.youtube.com/watch?v=PJjmw9TRB7s and lots of blogs …

Code reviews as a regular thing every day? Who is doing code reviews? Who is enjoying them? Who is doing them because you have to?

Good code is like a joke! No need for explanation

Why do we do Code reviews?

“Code Reviews make me better every day.“ ● Knowledge transfer ● Increased team awareness ● Finding alternative solutions

The process is more important than the result. Discuss your code with your peers.

What is a strong Code Review culture? Everybody involved. Not seniors over juniors. Don’t dismiss feedback, healthy discussion, why this question?

What is a strong Code Review culture? There is a negativity bias with written communication. PRs. How do we handle disagreements? What should I be reviewing?

1. As an author... Provide sufficient context. Content is king ? Context is God -> Why? What problems are we solving. Commit message. Link to Jira? No. What have you learned? Add 2 paragraphs of context.?

2. As a reviewer Ask don't tell. “What do you think…” “Did you consider…” “Can you clarify…”

How to deal with conflict? Don’t just agree on the issue: --> Agree to disagree + Commit Fear of conflict → Commitment Exception? We don’t agree on the process? eg “Commit directly to master”?

What to review? Timing: Small changes -> easier to provide context ● SRP ● ● Naming (2 hard things in CS: Naming, Cache invalidation, timezones) Complexity: shape of change = complex ● Test coverage ● Styleguide: Agree and outsource the tool. A bot tells you

Refactoring PR Give context on refactor PR Why this refactoring?

Strong Code review culture leads to ● Better code ● Better developers ● Team ownership ● Healthy debate > silent agreement

Review VS Pairing A Code Review = async pairing Async advantage → Change stands on its own!! Time zone differences?

Tools Tools: Pull Requests + Github, Gitlab TFS code-review Codeclimate, upsource/gerrit, static code analyzers, stylecop/SonarQube/Sputnik

Review not only code ● Tests ● Build process ● Deployment scripts ● Configuration

</ Code Review Tips

Code Review Smells

Management follows PRs

Authors merge own PR

The tech lead is the gatekeeper

</ Code Review Smells

What else? #codereview channel on SC Slack https://softwarecraftsmanship.slack.com/messages/code- review/ Invite via http://scslackin-rradczewski.rhcloud.com/

Feedback door

Next SCZ session? Do you want to run a workshop? Do you have a topic? → Get in touch! What about: How to spice up your Code Review?

What will you do different tomorrow? @peitor Continue the conversation @patbaumgartner