TECH TALK 9 MOJE SPOJRZENIE NA CODE REVIEW

Moje spojrzenie na udane code review

Code review - czym jest?

Proces techniczno - społeczny(!?), będacy formą statycznej analizy kodu, mający na celu wykrycie błędów, tzw. "smrodków w kodzie" (ang. code smells), rozbieżności z przyjętym stylem, a także okazją do ewaluacji poziomu kodowania i dania informacji zwrotnej.

Code review - czym nie jest?

Okazją do udowodnienia, że:

Ja wiem wszystko - "Zobacz jaki jestem zajedwabisty!"
Ty nie wiesz nic - "To co napisałeś jest tak słabe, że jak patrzę to dostaję, migotania przedsionków"

Czynności wstępne

Konwencje

Czy w danym języku są przyjęte jakieś standardy/konwencje/zalecenia odnośnie stylu/ sposobu pisania kodu (jak pisać nazwy zmiennych, klas, itp; indentacji; strukturyzowania plików źródłowych; itd.)? Przykłady: PEP8, Zen of Python
Czy zespół posiada własny kodeks (coding style)? Przykłady: Python Coding Style by Concent Team

Znajomość problemu

Żeby dobrze zreview-ować kod, muszę wiedzieć jaki problem stara się on rozwiązać. Jeśli jest podlinkowane issue / dokumentacja / blueprint - staram się z nią zapoznać i zrozumieć. Ciężko ocenić czy logika biznesowa jest zaimplementowana poprawnie jeśli jej nie znam / nie rozumiem ;)

Automaty

Czas programisty bywa dość drogi, więc jeżeli da się zautomatyzować jego pracę - należy to zrobić. Dlatego warto korzystać z narzędzi do statycznej analizy kodu (ang linters). Dzięki temu, jako reviewujący mogę skupić się bardziej na logice biznesowej, a mniej na "spacjach".

Review

Przykłady

Jeden obraz wart jest tysiąca słów. Kilka linijek kodu, może lepiej wytłumaczyć moje sugestie niż wielozdaniowa rozprawka. Przykłady: 1 2

Zaufanie

Nawet jeśli mam uwagi do kodu, nie zawsze używam opcji request changes. Robię to tylko wtedy, gdy z całą pewnością wiem, że będę chciał ponownie obejrzeć kod po poprawkach. Jeśli zmiany są niewielkie albo jestem pewny, że osoba, która ma je zrobić poradzi sobie z tym bez "nadzorcy", klikam approve changes.

Przykłady: 1 2

Oszczędzać czas - swój i innych

Staram się robić review sprawnie i najszybciej jak się da. Ograniczam liczbę "rund" do minimum. Ich duża ilość nie tylko zajmuje dużo czasu, ale również nie poprawia morale (zarówno autora PR-a jak i review-era).

Docenić dobre rozwiązania

Review jest bardzo dobrą okazją do dania komuś feedback-u. Komentarze do kodu nie muszą zawierać jedynie uwag krytycznych - jeśli widzę fajne/sprytne rozwiązanie staram się zostawić chociaż "łapkę w górze" ;)

Przykłady: 1

Zostawić możliwość decyzji

Nie wszystko w programowaniu jest czarno-białe. Staram się nie pozwolić, żeby moje osobiste preferencje (w sprawach niedogmatycznych) stawały się "prawdą objawioną". Zostawianie możliwości wyboru podnosi morale ;)

Przykłady: 1

Reviewować zmiany

Czasami widzę, że jest jakiś błąd/coś do poprawienia w kodzie, który leży obok kodu, który został zmodyfikowany. Uważam za nietakt wymagać, żeby autor PR-a musiał to zmienić. Jeśli zmiany są małe - proszę/pytam czy może to zrobić w ramach tego PR-a. Jeśli nie - robię (albo proszę o zrobienie) osobne issue.

Forma grzecznościowa

Słowa mają znaczenie. Staram się, żeby moje uwagi nie miały formy "oskarżającej". Preferuję zwroty typu "could we have it...", "please move it to...", "rasie -> raise", "please extract it to...", "I would suggest..."