Code review - jak krytykować

Maciej Iwanowski

O czym

  • o historii
  • o tym po co
  • o tym jak
  • o problemach
  • o korzyściach
Henry Oldenburg
  • Michael Fagan i IBM
  • Fagan Inspection Process
  • XP i Continunous Inspection
  • Agile Manifesto i jakość

Po co?

  • komunikacja
  • wczesne eliminowanie błędów
  • redukcja liczby błędów zgłaszanych przez klientów
  • identyfikowanie problemów w procesie

Lekkie przeglądanie

  • Cisco Sytems
  • 50 programistów, 2500 przejrzeń, 3.2 miliona wierszy kodu
  • do 200-400 wierszy kodu
  • do 300-500 wierszy na godzinę
  • do 60-90 minut
  • 15 błędów na godzinę
  • przygotowanie kodu do review

Lekkie przeglądanie

  • 90 minut jest wartością uznaną
  • 6 razy wydajniej
  • brak danych do porównania liczby błędów
  • wspomaga częste zmiany

Tak rób

  • przygotuj się
  • pisz testy
  • przeglądaj małe fragmenty
  • używaj narzędzi
  • szukaj defektów
  • pomyśl o czekliście

Tak nie rób

Nie bądź ignorantem

Nie bądź ignorantem

Tak nie rób

Nie wywyższaj się

Nie wywyższaj się

Tak nie rób

Nie formułuj bezsensownych żądań

Nie formułuj bezsensownych żądań

Tak nie rób

Szanuj partnerów

Szanuj partnerów

Problemy

  • 20%-30% dłużej
  • opór przed nowym i nieznanym
  • opór ze strony menadżerów
  • opór ze strony programistów

Korzyści

accounting
  • komunikacja, głupcze!
  • oszczędności
  • dotrzymywanie terminów
  • efektywniejsze testowanie

?

Code review - how to criticize

Maciej Iwanowski

About

  • history
  • goals
  • how-to
  • issues
  • benefits
Henry Oldenburg
  • Michael Fagan and IBM
  • Fagan Inspection Process
  • XP and Continunous Inspection
  • Agile Manifesto and quality

What for?

  • communication
  • eliminate bugs early
  • reduce number of bugs reported by customers
  • identifying issues in a process

Lightweight reviewing

  • Cisco Sytems
  • 50 developers, 2500 inspections, 3.2 million lines of code
  • up to 200-400 LoC
  • up to 300-500 LoC per hour
  • up to 60-90 minutes
  • 15 bugs per hour
  • preparing code for inspection

Lightweight reviewing

  • 90 minutes is widely accepted value
  • 6 times more efficient
  • impossible to compare number of bugs per 1000 LoC
  • supports frequent changes

Dos

  • prepare yourself
  • write tests
  • review small changesets
  • use tools
  • look for defects
  • think about a checklist

Don'ts

Don't be ignoramus

Don't be ignoramus

Don'ts

Nie wywyższaj się

Don't swagger

Don'ts

Don't talk rubbish

Don't talk rubbish

Don'ts

Respect partners

Don't disrespect partners

Issues

  • 20%-30% longer development time
  • resist against new and uknown
  • resisting managers
  • resisting developers

Benefits

accounting
  • communication, stupid!
  • saving
  • delivering on time
  • more effective testing

?