Jak wyrazić opinię po procesie przeglądu kodu

10

Obecnie sprawdzam kod młodszych programistów, którzy właśnie dołączyli do mojego zespołu. Zastanawiam się, jak powinienem dostarczyć wyniki tej recenzji:

  1. Czy powinienem sam naprawić kod?

  2. Czy powinienem przekazać im informacje zwrotne na temat procesu sprawdzania i pozwolić im wykonać poprawki zgodnie z moimi instrukcjami? A jeśli tak, to w jaki sposób mogę przekazać tę informację zwrotną, czy wypełnię określony szablon dokumentu i wyślę go do nich, czy też istnieje oprogramowanie, które pomoże mi oznaczyć problemy w plikach kodu, w których mogą je później sprawdzić? (Używam Visual Studio).

Po zakończeniu sprawdzania kodu i poprawek, upłynęło trochę czasu, a niektóre części kodu, który sprawdziłem w przeszłości, zmieniły się, w jaki sposób mogę przeprowadzić proces ponownej oceny? Czy powinienem ponownie sprawdzić cały kod? A może po prostu sprawdzam części, które uległy zmianie? A jeśli tak, to w jaki sposób mogę śledzić zmienione części, aby uniknąć podwójnego sprawdzania kodu?

Syzyf
źródło
1
Możliwy duplikat Jak należy przeprowadzać recenzje kodu?
t3chb0t

Odpowiedzi:

14

Krótka odpowiedź

Czy powinienem sam naprawić kod?

Nie.

Czy powinienem przekazać im informacje zwrotne na temat procesu sprawdzania i pozwolić im wykonać poprawki zgodnie z moimi instrukcjami?

Tak. Zgodnie z twoimi sugestiami , a nie instrukcjami . Instrukcje wydają się zbyt wiarygodne.

A jeśli tak, to w jaki sposób mogę przekazać tę informację zwrotną, czy wypełnię określony szablon dokumentu i wyślę go do nich, czy też istnieje oprogramowanie, które pomoże mi oznaczyć problemy w plikach kodu, w których mogą je później sprawdzić? (Używam Visual Studio).

Użyj narzędzia, aby przekazać opinię. Możesz używać programu Visual Studio.

Długa odpowiedź

Kiedyś korzystałem z Visual Studio, ale ciągle musiałem pytać innych programistów: „Hej, możesz wysłać mi swoją pracę, abym mógł ją przejrzeć?” Nie podobało mi się to i nigdy nie wyszło tak dobrze. Teraz używam Asystenta recenzji, ponieważ mogę rozpocząć recenzję, sprawdzając checkins. Nie muszę polegać na tym, że inny programista przysłał mi pracę do sprawdzenia. Pomaga mi także oznaczać przedmioty jako defekty lub po prostu oznaczać przedmioty, na które autor ma się zwrócić. Działa to dla naszego zespołu, ponieważ po rozpoczęciu recenzji, pozostaje na tablicy przeglądowej i nie gubi się w tłumaczeniu. Jest to zintegrowane z Visual Studio. Jak wspomniałem, Visual Studio ma również swój natywny proces recenzji, ale uważam, że ma ograniczenia i proces ten nie jest naturalny; dlatego używam Asystenta przeglądu.

To narzędzie pomaga również w procesie tam iz powrotem, dyskusjach itp.

Proces ten jest mniej więcej następujący:

Coś recenzuję, a następnie wysyłam do autora (w twoim przypadku młodszego autora). Dokonują zmian, a następnie odsyłają. Patrzę na zmiany i wyrażam opinię. Jeśli jestem zadowolony ze zmian, zamykam recenzję. W przeciwnym razie idzie tam iz powrotem. Czasami, jeśli jest zbyt wiele tam iz powrotem, po prostu podchodzę do ich biurka i używam tablicy - to naprawdę przyspiesza ten proces.

Recenzje kodu są wrażliwym obszarem, więc bądź bardzo ostrożny przy wyborze sformułowań. Nigdy nikomu nie mówię

Zły wybór sformułowania

Przejrzałem Twój kod i jest kilka elementów, które musisz zmienić.

Zamiast tego mówię to:

Lepszy wybór brzmienia

Spojrzałem na twój kod i potrzebuję pomocy. Czy możesz przejrzeć elementy, które ci wysłałem i sprawdzić, czy możesz wyjaśnić niektóre z moich pytań?

To sprawia, że ​​autor myśli:

  1. Potrzebuję pomocy, żeby nie przeszli w tryb obronny.
  2. Wygląda na to, że to recenzent, a nie ja. Technicznie rzecz biorąc, skoro proszę ich o inne spojrzenie i zmianę niektórych rzeczy, są jakby recenzentem.

Te proste wybory słów ogromnie mi pomogły.

Nigdy nie lekceważę młodszych programistów. Pracowałem z kilkoma starszymi programistami (ponad 10 lat doświadczenia) i tam kod był gorszy niż młodszy student w trybie współpracy. Więc to, że są starsi lub młodsi, nie jest aż tak ważne; ich praca mówi naprawdę głośniej niż lata doświadczenia.

Często, aby zachęcić młodszych deweloperów i zachęcić ich do udziału w recenzjach, wyślę im coś do recenzji: coś, co mogą wymyślić, coś, co będzie dla nich wyzwaniem, ale nie do końca. Mogę to sformułować jak poniżej:

Czy możesz przejrzeć dla mnie kod, ponieważ nie mogę go rozgryźć.

To mi bardzo pomaga. Pomaga to, ponieważ jasno pokazuje, że nie tylko ja recenzuję, ale także robią recenzje i są również częścią tego procesu. To pokazuje, że cała idea polega na stworzeniu dobrego, czystego kodu i w razie potrzeby poprosić o pomoc. Proces recenzji to kultura, więc naprawdę musimy nad tym popracować.

Teraz niektórzy ludzie mogą się martwić, że jeśli to zrobią, młodsi twórcy stracą szacunek, ponieważ po prostu zrobili coś, czego nie moglibyście zrobić. Ale to dalekie od prawdy: prośba o pomoc pokazuje pokorę. Ponadto istnieje wiele sytuacji, w których możesz zabłysnąć. Wreszcie, jeśli to jest twój strach, masz problemy z samooceną. Wreszcie, może naprawdę tego nie wiem: mam na myśli, że niektórzy z tych deweloperów mają w głowie nowe algorytmy, ponieważ właśnie studiowali je miesiąc temu.

W każdym razie wracając do juniorów i recenzji. Powinieneś zobaczyć ich miny, kiedy się zorientują i wyślą mi odpowiedź. Następnie mogę im powiedzieć: „OK, pozwól mi dokonać zmian, a jeśli jesteś zadowolony, zamknij problem”.

Czują się wspaniale, że mogą spojrzeć na moją pracę i powiedzieć: „Tak, twoje zmiany są dobre. Zamknąłem problem”.

Sam nigdy nie naprawiam kodu, ponieważ:

  1. Autor nie będzie się z tego uczył.
  2. To tak, jak mówię: „Odsuń się, pozwól, że pokażę ci, jak to się robi. Mój kod jest lepszy niż Twój”.
  3. Dlaczego miałabym? To dla mnie więcej pracy.

Ale w komentarzach zasugeruję pomysły i fragmenty kodu, aby pomóc autorowi. Pamiętaj, że czasami moje recenzje po prostu pytają autora, że ​​nie rozumiem ich kodu; w ich kodzie nie może być nic złego. Być może będą musieli zmienić nazwy zmiennych, dodać komentarze itp. Dlatego nie będę nawet wiedział, co zmienić w takim przypadku; tylko oni będą.

Jeśli będziesz robił recenzje, wcześniej czy później będziesz miał naprawdę dobry pomysł na poziom wiedzy każdego programisty w zespole. Wiedza o tym jest naprawdę przydatna i musisz z niej skorzystać i ją uwolnić. Oto jak: jeśli przejrzę jakiś kod i zobaczę oczywiste obszary wymagające poprawy, i wiem, że inny programista też może je złapać, poproszę ich o sprawdzenie. Coś w stylu „Hej, widzę kilka obszarów, które można poprawić. Czy możesz przejrzeć to bardziej szczegółowo i wysłać swoje uwagi autorowi?” To też działa świetnie, ponieważ teraz mam 2 innych programistów pracujących ze sobą.

Jeśli recenzuję trochę pracy i zauważę coś, z czego cały zespół może skorzystać, to stworzę hipotetyczny scenariusz i wyjaśnię ten problem na spotkaniu. Zacznę od wyjaśnienia scenariusza i zapytam wszystkich, czy mogą znaleźć problem lub zobaczyć problem, zaangażować go. Spraw, aby wszyscy zadawali pytania. Następnie zaprezentuj lepsze podejście. Jeśli ktoś ma lepsze podejście, dziękuję mu i potwierdzam przed zespołem, że jego podejście jest lepsze. To pokazuje, że nie jestem osobowością typu „moja droga lub autostrada”. Ponadto NIGDY nie otwieram czyjejś pracy i nie zaczynam wskazywać problemów na spotkaniu, autor nie doceni tego - niezależnie od tego, jak mi miło i nieszkodliwie jestem.

Kiedy robię recenzje, szukam nie tylko dobrego, czystego kodu, ale także jego działania. Jeśli wymaganiem biznesowym jest: Jeśli pracownik jest w firmie dłużej niż 10 lat, daj mu 5% wzrostu. W przeciwnym razie 2,5% . Pierwszą rzeczą, którą sprawdzam, jest to, czy faktycznie to robi. Następnie sprawdzam, czy robi to w czysty, spójny i wydajny sposób.

Jeśli dokonam recenzji, upewnię się, że ją skontaktuję, inaczej nikt nie potraktuje jej poważnie.

Kiedyś pracowałem z kimś przed laty, który przeprowadzałby recenzję i pracował jako kierownik programisty oraz kierownik ds. Kontroli jakości, ale obaj menedżerowie wywodzili się ze środowisk biznesowych lub mieli niewielkie doświadczenie programistyczne. Zrobił to tylko po to, aby zrobić na nich wrażenie. Nikt tego nie lubił i wtedy powiedziałem sobie, że nigdy nie popełnię tego błędu.

Inną rzeczą, którą robił, było wybieranie stylu programowania i był przekonany, że jego styl jest najlepszy („Mój kungfu jest o wiele lepszy niż twój małpi styl…”). To była dla mnie kolejna lekcja: zawsze istnieje więcej niż jeden sposób rozwiązania problemu.

Odpowiedz na niektóre z ponumerowanych pytań

1- czy powinienem samodzielnie naprawić kod?

Nie, proszę zobaczyć powody, które podałem powyżej.

2- czy powinienem przekazać im informacje zwrotne na temat procesu recenzji i pozwolić im wykonać poprawki zgodnie z moimi instrukcjami?

Tak, spróbuj użyć zdań i tonu, który jest przyjazny, ale wymaga uwagi. Bądź tak jasny, jak to tylko możliwe. Podaj, na czym polega problem z kodem, jak go poprawić. Nie proś tylko o zmianę. Ale podaj powody.

jak przekazać tę informację zwrotną, czy wypełnić jakiś dokument szablonu i wysłać go do niego, czy też jest jakieś oprogramowanie, które pomoże mi oznaczyć problemy w plikach kodu, w których mogą później je sprawdzić? (Używam Visual Studio).

Tak jak powiedziałem, możesz użyć narzędzia, którego używam, lub innego narzędzia. Nie używaj wiadomości e-mail ani dokumentów słownych, ponieważ gubią się i trudno jest je śledzić.

Po zakończeniu sprawdzania kodu i poprawek, które minęły, minęło trochę czasu, a niektóre części kodu, który sprawdziłem w przeszłości, zmieniły się, jak mam przeprowadzić proces ponownej oceny? czy powinienem ponownie sprawdzić cały kod?

Przeważnie sprawdzam różnicę (tylko zmiany). Ale musisz mieć na uwadze ogólny obraz, aby mieć pewność, że nic się nie zepsuje i będzie zgodne z architekturą.

Końcowe przemyślenia

Osobiście uważam, że słowo „przegląd kodu” jest złym wyborem i nie wiem, jak się zaczęło. Mogli wybrać lepsze i mniej wiarygodne słowo.

Kodowanie Yoshi
źródło
Działa to tylko w przypadku drobnych rzeczy, takich jak nazwy zmiennych. Jednak jeśli architektura jest zła, nie ma narzędzia, które może pomóc. Wystarczy wyrzucić całą rzecz i przepisać ją.
t3chb0t
@ t3chb0t Dlaczego małe rzeczy? Przez making sense architecturally, miałem na myśli upewnienie się, że kod warstwy dostępu do danych znajduje się w warstwie dostępu do danych, sprawdzanie poprawności interfejsu użytkownika w interfejsie użytkownika itp. Innymi słowy, upewnienie się, że obawy nie zostaną przeniesione do innych obszarów. Istnieją narzędzia pomocne w utrzymaniu architektury; faktycznie VS robi to teraz po wyjęciu z pudełka . My również tego używamy.
CodingYoshi,
Jeśli chodzi o narzędzia, uważam, że doskonałym narzędziem do użycia jest dowolna forma oprogramowania do sprzedaży biletów / śledzenia pracy, której prawdopodobnie już używasz do śledzenia tego, co należy zrobić. Na przykład, jeśli korzystasz z Github, możesz mieć wszystkie zmiany związane z problemem, a następnie sprawdzenie zostanie wykonane w tym wątku dyskusji. Jira i Trac to dwa inne takie narzędzia. Utrzymuje to scentralizowane miejsce na wszystkie informacje związane z pracą i jest wyraźnie widoczne przed zamknięciem biletu.
Kat
@Kat Nie jestem pewien, czy narzędzie do sprzedaży biletów jest właściwym narzędziem do użycia tutaj. Narzędzia do przeglądu kodu pokazują różnice między zmianami, a problemy są inne niż problemy w systemie sprzedaży biletów; mają na myśli różne rzeczy. Ale mogę się mylić.
CodingYoshi
3

Wiele zależy od tego, jak rozumiesz recenzje kodu w Twojej firmie. Są firmy, w których przegląd kodu jest wysoce sformalizowanym procesem, który odbywa się rzadko, ale jest dużą sprawą. W innych przypadkach przegląd kodu jest obowiązkową częścią każdego zadania, które jest realizowane, i jest bardzo przyziemną i szybką rzeczą przy niewielkim formalizmie. Osobiście wybieram to drugie podejście, ale to może być Twoja decyzja, czy możesz z niej skorzystać, czy nie.

Napisałem wpis na blogu zatytułowany Czy recenzje kodu są warte twojego czasu? podsumowując proces stosowany przez mój zespół. Na wynos, ponieważ odnoszą się do twojego pytania, byłyby:

  1. Pozwól programistom naprawić kod. Pozwoli im to lepiej zrozumieć twoje komentarze (lub uświadomią sobie, że nie rozumieją ich w pełni i zapytają), a wykonanie zadania jest lepszym sposobem na naukę niż tylko czytanie o nim.
  2. Oprogramowanie do przeglądów kodów to droga. Dostępnych jest wiele opcji, zarówno typu open source, jak i zastrzeżonych. Większość z nich działa z git. Mój zespół używa BitBucket (wcześniej znany jako Stash), są też GitLab i GitHub, Gerrit (których osobiście nie jestem fanem) i kilka innych. Większość tych aplikacji jest oparta na sieci Web, więc nie ma znaczenia, jakiego IDE używasz, ale są też wtyczki do wielu IDE, więc jestem pewien, że są też pewne dla Visual Studio. Takie oprogramowanie pozwala przejrzeć kod przed scaleniem z główną gałęzią (zwykle za pomocą poleceń Pull) i pokazuje zmodyfikowane części i niektóre linie kontekstu wokół każdej zmiany. Dzięki temu przegląd kodu jest płynny i bezproblemowy.

Jeśli chodzi o cykl sprawdzania-sprawdzania-sprawdzania-poprawiania, to, co wybierzesz, powinno zależeć od dojrzałości programistów i wagi wykrytych problemów. Gdy zespoły dokonują codziennych przeglądów kodu, możesz spodziewać się, że proste zmiany, takie jak zmiana nazwy klasy, zostaną zastosowane i prawdopodobnie nie będzie potrzeby ich ponownego sprawdzania. Jeśli zespół nie został jeszcze sprzedany w recenzjach kodu lub ludzie są naprawdę niedoświadczeni, możesz chcieć sprawdzić takie rzeczy bez względu na to. Z drugiej strony, jeśli twoja recenzja zauważyła złożony problem współbieżności, którego młodsi deweloperzy mogą nie w pełni zrozumieć, nawet po tym, jak im to wskazałeś, lepiej sprawdź poprawkę i nie zatwierdzaj zmiany, zanim upewnisz się, że została naprawiona.

Narzędzia mogą ci w tym pomóc, ponieważ jeśli pracujesz z żądaniami ściągania, możesz łatwo skonfigurować oprogramowanie, aby nie zezwalało na łączenie zmian, dopóki nie uzyska zgody określonej liczby programistów. Zwykle można przeglądać zmiany w poszczególnych zatwierdzeniach zmiany, co pozwala łatwo przeglądać tylko zmiany dodane od ostatniej rundy komentarzy.

Michał Kosmulski
źródło
1

Głosuję za drugą opcją. Juniorzy mogą mieć lepszą „krzywą ciągłości” podczas samodzielnego wprowadzania zmian.

Ponadto: nie bądź jedynym, który przegląda kod.
Pozwól niektórym doświadczonym członkom twojego zespołu również przyjrzeć się kodowi i zaplanować regularne spotkanie przeglądowe, podczas którego recenzenci przedstawią autorowi swoje ustalenia (dokonane przed spotkaniem!). Zwiększy to motywację zarówno juniorów, jak i członków zespołu doświadczeń.

Timothy Truckle
źródło
4
Świetny punkt Również juniorzy powinni „przejrzeć” kod PO i kod drugiej strony.