Po poważnych problemach z jakością w ostatnim roku moja firma niedawno wprowadziła recenzje kodów. Proces przeglądu kodu został szybko wprowadzony, bez wytycznych i jakiejkolwiek listy kontrolnej.
Wybrano mnie i innego programistę do przeglądu wszystkich zmian dokonanych w systemach, zanim zostaną one scalone z bagażnikiem.
Zostaliśmy również wybrani jako „Technical Lead”. Oznacza to, że jesteśmy odpowiedzialni za jakość kodu, ale nie mamy żadnych uprawnień do wprowadzania zmian w procesie, ponownego przypisywania programistów lub wstrzymywania projektów.
Technicznie możemy odrzucić połączenie, przywracając je do rozwoju. W rzeczywistości kończy się to prawie zawsze, gdy nasz szef żąda dostarczenia go na czas.
Nasz menedżer jest MBA, który zajmuje się głównie tworzeniem harmonogramu nadchodzących projektów. Podczas gdy próbuje, prawie nie ma pojęcia, co robi nasze oprogramowanie z biznesowego punktu widzenia, i stara się zrozumieć nawet najbardziej podstawowe wymagania klientów bez wyjaśnienia ze strony dewelopera.
Obecnie rozwój odbywa się w oddziałach programistycznych w SVN, po tym, jak deweloper myśli, że jest gotowy, ponownie przypisuje bilet w naszym systemie biletowym do naszego menedżera. Menedżer następnie przypisuje nam to.
Recenzje kodu spowodowały pewne napięcia w naszym zespole. Zwłaszcza niektórzy starsi członkowie kwestionują zmiany (tj. „Zawsze tak robiliśmy” lub „Dlaczego metoda powinna mieć sensowną nazwę, wiem, co robi?”).
Po kilku pierwszych tygodniach moja koleżanka zaczęła się przesuwać, aby nie sprawiać kłopotów współpracownikom (powiedziała mi, że po zgłoszeniu błędu przez klienta, wiedziała o błędzie, ale obawiała się, że deweloper byłby na nią zły za to, że to wskazała).
Z drugiej strony jestem teraz znany z tego, że jestem dupkiem, który wskazuje problemy z popełnionym kodem.
Nie sądzę, że moje standardy są zbyt wysokie.
Moja lista kontrolna w tej chwili to:
- Kod się skompiluje.
- Jest co najmniej jeden sposób działania kodu.
- Kod będzie działał z większością normalnych przypadków.
- Kod będzie działał z większością przypadków skrajnych.
- Kod zgłosi uzasadniony wyjątek, jeśli wstawione dane są nieprawidłowe.
Ale w pełni akceptuję sposób, w jaki udzielam informacji zwrotnej. Podaję już praktyczne punkty wyjaśniające, dlaczego coś należy zmienić, czasem nawet po prostu pytam, dlaczego coś zostało zaimplementowane w określony sposób. Kiedy myślę, że jest źle, zwracam uwagę, że opracowałbym go w inny sposób.
Brakuje mi umiejętności znalezienia czegoś, co można by nazwać „dobrym”. Czytałem, że należy próbować przekazywać złe wiadomości w dobrych wiadomościach.
Ale trudno mi znaleźć coś dobrego. „Hej, tym razem faktycznie popełniłeś wszystko, co zrobiłeś” jest bardziej protekcjonalne niż miłe lub pomocne.
Przykładowy przegląd kodu
Hej Joe,
Mam pytania dotyczące twoich zmian w klasie Library \ ACME \ ExtractOrderMail Class.
Nie rozumiem, dlaczego oznaczyłeś „TempFilesToDelete” jako statyczny? W tej chwili drugie wywołanie „GetMails” wyrzuciłoby wyjątek, ponieważ dodajesz do niego pliki, ale nigdy ich nie usuwasz po ich usunięciu. Wiem, że funkcja jest wywoływana tylko raz na przebieg, ale w przyszłości może się to zmienić. Czy możesz po prostu uczynić go zmienną instancji, wtedy moglibyśmy mieć wiele obiektów równolegle.
... (Niektóre inne punkty, które nie działają)
Drobne punkty:
- Dlaczego „GetErrorMailBody” przyjmuje wyjątek jako parametr? Przegapiłem coś? Nie rzucasz wyjątku, po prostu przekazujesz go i wywołujesz „ToString”. Dlaczego?
- SaveAndSend To nie jest dobra nazwa dla metody. Ta metoda wysyła wiadomości z błędami, jeśli przetwarzanie wiadomości nie powiodło się. Czy możesz zmienić nazwę na „SendErrorMail” lub coś podobnego?
- Nie komentuj starego kodu, usuń go całkowicie. Nadal mamy to w obaleniu.
źródło
Odpowiedzi:
Świetnie, masz prawdziwą okazję do stworzenia wartości dla swojej firmy.
Twój współpracownik nie powinien dokonywać przeglądu kodu, jeśli nie jest w stanie poradzić programistom, co jest nie tak z ich kodem. Twoim zadaniem jest znaleźć problemy i rozwiązać je, zanim wpłyną one na klientów.
Podobnie programista zastraszający współpracowników prosi o zwolnienie. Po przeglądzie kodu poczułem się zastraszony - powiedziałem mojemu szefowi i wszystko zostało załatwione. Poza tym lubię swoją pracę, więc podtrzymywałem pozytywne i negatywne opinie. Jako recenzent to mnie nie dotyczy.
Cóż, to niefortunne, mówisz, że jesteś taktowny. Możesz znaleźć więcej do pochwały, jeśli masz więcej do znalezienia.
Krytykuj kod, a nie autora
Podajesz przykład:
Zamiast tego używaj słów „ty” i „twój”, powiedzmy, „zmiany”.
Nie dodawaj retorycznych zawijasów do swoich krytyków. Nie rób też żartów. Słyszę zasadę: „Jeśli sprawia ci przyjemność mówienie, nie mów tego, to nie jest dobre”.
Może wzmacniasz własne ego kosztem kogoś innego. Zachowaj to tylko do faktów.
Podnieś poprzeczkę, przekazując pozytywne opinie
Podnosi poprzeczkę, aby chwalić innych programistów, którzy spełniają wyższe standardy. To oznacza pytanie
jest dobry i wart uwagi.
Możesz wskazać, gdzie kod spełnia idee praktyk kodowania wyższego poziomu.
Szukaj ich, aby postępowali zgodnie z najlepszymi praktykami i wciąż podnosili poprzeczkę. Po tym, jak wszyscy będą oczekiwać łatwiejszych ideałów, przestaniesz je chwalić i poszukasz jeszcze lepszych praktyk kodowania.
Najlepsze praktyki dotyczące języka
Jeśli język obsługuje dokumentację w kodzie, przestrzeniach nazw, obiektowych lub funkcjonalnych funkcjach programowania, możesz wywołać je i pogratulować autorowi używania ich w stosownych przypadkach. Te kwestie zwykle należą do przewodników po stylach:
Ogólne najlepsze praktyki
W różnych paradygmatach można znaleźć punkty do pochwały na temat ogólnych zasad kodowania. Na przykład, czy mają dobre unittests? Czy Unittests obejmują większość kodu?
Szukać:
Programowanie funkcjonalne
Jeśli język jest funkcjonalny lub obsługuje funkcjonalny paradygmat, poszukaj następujących ideałów:
Programowanie obiektowe (OOP)
Jeśli język obsługuje OOP, możesz pochwalić odpowiednie użycie tych funkcji:
w ramach OOP istnieją również SOLIDNE zasady (być może pewna redundancja funkcji OOP):
Zasady programowania w Uniksie :
Zasady uniksowe to modułowość, klarowność, skład, separacja, prostota, oszczędność, przejrzystość, solidność, reprezentacja, najmniej zaskoczenie, cisza, naprawa, oszczędność, generowanie, optymalizacja, różnorodność i rozszerzalność.
Zasadniczo zasady te można zastosować w wielu paradygmatach.
Twoje kryteria
Są to zbyt trywialne - czułbym się pochlebny, gdyby mnie za to pochwalono:
Z drugiej strony są to dość wysokie pochwały, biorąc pod uwagę to, z czym masz do czynienia, i nie zawahałbym się pochwalić programistów za to:
Zapisujesz zasady przekazywania recenzji kodu?
To świetny pomysł w teorii, chociaż chociaż zwykle nie odrzucałbym kodu z powodu złego nazewnictwa, widziałem tak złe nazewnictwo, że odrzuciłbym kod z instrukcjami, jak to naprawić. Musisz mieć możliwość odrzucenia kodu z dowolnego powodu.
Jedyną regułą, o której myślę o odrzuceniu kodu, jest to, że nie ma nic tak rażącego, że nie chciałbym go produkować. Naprawdę złe imię to coś, co chciałbym powstrzymać od produkcji - ale nie możesz tego zrobić.
Wniosek
Możesz pochwalić stosowanie najlepszych praktyk pod wieloma paradygmatami i prawdopodobnie pod wszystkimi nimi, jeśli język je obsługuje.
źródło
Nie zawracaj sobie głowy wybieraniem czegoś dobrego, chyba że jest to solidny zwięzły przykład i nie jest bezpośrednio związany z głównym tematem.
Nie powielę tego - z dźwięków tego masz do czynienia z co najmniej jedną osobą, która nie jest pewna swoich umiejętności i niedojrzale radzi sobie z wyzwaniami związanymi z ich pracą. Prawdopodobnie są również źli w swojej pracy - dobry programista powinien zawsze być skłonny do refleksji, konstruktywnej krytyki i otwartości na zmiany.
Teraz, gdy jest w powietrzu, porozmawiajmy o tobie. Niezależnie od tego, czy uważasz, że jesteś rozsądny, musisz być bardzo ostrożny z ludźmi takimi jak ten, aby uruchomić piłkę. Znalazłem najlepszy sposób na radzenie sobie z tymi ludźmi, aby być bardzo ostrożnym z tym, jak wypowiadasz słowa.
Upewnij się, że obwiniasz kod, a nie autora . Skoncentruj się na jednym z omawianych problemów, a nie na kupie-górze, która jest twoją bazą kodu, którą mogli mieć znaczący udział w tworzeniu i będą postrzegani jako dalszy osobisty atak. Na początku wybierz swoje bitwy, skup się na krytycznych kwestiach, które ujawniają się twoim użytkownikom, aby nie wywoływać salwy krytyki u osoby, która prowadzi ich do odrzucenia wszystkiego, co mówisz.
Język ciała i ton są ważne, jeśli rozmawiasz z nimi osobiście, jasno mów o tym, co mówisz i upewnij się, że nie rozmawiasz z nimi ani nie lekceważysz ich umiejętności technicznych. Najprawdopodobniej od razu staną w defensywie, więc musisz rozwiązać ich zmartwienia zamiast je potwierdzić. Musisz być świadomy tej rozmowy, nie będąc zbyt oczywistym, aby podświadomie myśleli, że jesteś po ich stronie, i miejmy nadzieję, że zaakceptują konieczność wprowadzenia zmian, na które zwrócono uwagę.
Jeśli to nie zadziała, możesz zacząć być bardziej agresywny. Jeśli produkt można uruchomić z sali konferencyjnej, przywołaj go do projektora podczas przeglądu kodu i pokaż błąd z pierwszej ręki, lepiej, jeśli jest tam menedżer, aby osoba nie mogła się wycofać. Nie chodzi o to, aby ich zawstydzić, ale zmusić ich do zaakceptowania, że problem jest prawdziwy dla użytkownika i że należy go naprawić, a nie tylko przeszkodą, jaką wywołuje jego kod.
W końcu, jeśli nigdzie się nie dostaniesz, jesteś zmęczony traktowaniem osoby jak ucznia w przedszkolu, a kierownictwo zupełnie nie zdaje sobie sprawy z problemu i albo źle wpływa na twoją wydajność jako recenzent kodu, albo zależy ci na dobrym samopoczuciu twojego firma i / lub produkt, musisz zacząć rozmawiać z szefem o ich zachowaniu. Bądź tak konkretny i bezosobowy, jak to możliwe - uzasadnij biznes, że cierpi wydajność i jakość.
źródło
Przeglądy kodów mogą być toksyczne, marnować czas, chęć rujnowania żywych wojen. Wystarczy spojrzeć na rozbieżność opinii na temat takich rzeczy, jak czysty kod a komentarze, konwencje nazewnictwa, testy jednostek i integracji, strategie sprawdzania, RESTfulness itp. Itp.
Jedynym sposobem na uniknięcie tego jest zapisanie zasad przekazywania recenzji kodu.
To nie jest osoba, która nie zgodzi się lub nie zatwierdzi meldowania. Sprawdzają jedynie, czy przestrzegano zasad.
A ponieważ są one zapisywane z góry, podczas pisania kodu możesz przestrzegać reguł i nie musisz wyjaśniać swojego rozumowania ani argumentów później.
Jeśli nie lubisz reguł, zwołaj spotkanie, aby je zmienić.
źródło
Nie chciałbym osłaniać twojej opinii, ponieważ można to uznać za protekcjonalne.
Moim zdaniem najlepszą praktyką jest zawsze koncentrowanie się na kodzie, a nigdy na autorze.
To recenzja kodu , a nie opinia programisty , więc:
Bardzo ważne jest również stosowanie tej samej reguły podczas rozmowy o recenzji z innymi:
Przegląd kodu nie jest czasem na sprawdzenie wydajności - należy to zrobić osobno.
źródło
Dziwi mnie, że do tej pory nie wspomniano w żadnej odpowiedzi i być może moje doświadczenie z recenzjami kodu jest niezwykłe, ale:
Dlaczego przeglądasz całe żądanie scalenia w jednej wiadomości?
Moje doświadczenia z recenzjami kodu są za pośrednictwem GitLab; Zawsze wyobrażałem sobie, że inne narzędzia do sprawdzania kodu będą działać podobnie.
Kiedy daję recenzję kodu, komentuję określone, pojedyncze linie diff. Jest to bardzo mało prawdopodobne, aby zostało odebrane jako osobista krytyka, ponieważ jest komentarzem do kodu - i faktycznie jest wyświetlane jako komentarz do kodu, pokazany bezpośrednio pod kodem, na którym jest komentarzem.
Mogę również komentować całe żądanie scalenia i często tak robię. Ale konkretne punkty można wskazać na konkretnych liniach różnicy. (Ponadto po dodaniu nowego zatwierdzenia w taki sposób, że zmienia się diff, komentarze do „przestarzałego diff” są domyślnie ukryte).
Dzięki temu przepływowi recenzji kodu stają się znacznie bardziej modułowe i mniej spójne. Wiersz z recenzji kodu może po prostu powiedzieć:
Lub może powiedzieć
Lub jeśli występują poważne problemy z sekcją, mogę napisać:
Po napisaniu wszystkich powyższych informacji mogę skomentować podsumowanie całego żądania scalenia, na przykład:
Nawet jeśli prośba o połączenie jest kompletnym śniadaniem dla psa, indywidualne komentarze mogą być proste. Będzie ich po prostu więcej. Następnie komentarz podsumowujący może powiedzieć:
Podsumowanie: Przejrzyj techniczne aspekty kodu jako komentarze do poszczególnych wierszy kodu. Następnie streść te komentarze w ogólnym komentarzu do żądania scalenia. Nie bądź osobisty - po prostu zajmuj się faktami i, według ciebie, kodem , a nie koderem. Opieraj swoją opinię na faktach, dokładnych obserwacjach i zrozumieniu.
źródło
Jestem naprawdę zaskoczony, że nikt tego nie zauważył, ale coś jest nie tak z opublikowaną recenzją próbki.
Po prostu nie ma powodu, aby zwracać się bezpośrednio do Joe. To, że Joe naprawia swoje niepowodzenia, nie jest twoją sprawą. To, że ktoś to robi, to twoja sprawa. Twoja sprawa dotyczy jakości kodu. Więc zamiast pisać prośby / zamówienia / żądania (że gdybym był Joe, mógłbym po prostu odmówić, ponieważ nie jesteś do tego uprawniony), pozostań przy swojej roli i napisz prostą anonimową listę rzeczy do zrobienia.
Próba uczciwego dawania dobrych i złych punktów jest nie tylko niemożliwa, ale całkowicie wykluczona z twojej roli.
Oto przykład przeformułowania z treścią zaczerpniętą z recenzji:
Jeśli sformułujesz taką recenzję, bez względu na to, jak bardzo czytelnik nienawidzi cię osobiście, wszystko, co widzi, to notatki na temat ulepszeń, które ktoś musi później wprowadzić. Kto Kiedy ? Nikogo to nie obchodzi. Co ? Dlaczego ? TO powinieneś powiedzieć.
Zajmiesz się właśnie przyczyną wzrostu napięcia w przeglądach kodu, usuwając czynnik ludzki z równania.
źródło
Celem przeglądu kodu jest znalezienie problemów. Jeśli wystąpi jakikolwiek błąd, najlepszą rzeczą, jaką możesz zrobić, to napisać niesprawny test.
Twój zespół (kierownik) powinien komunikować się, że produkcja błędów jest częścią gry, ale znalezienie i je naprawiać uratuje niesłyszących pracę.
Pomocne mogą być regularne spotkania (zespół lub para) i omówienie kilku zagadnień. Pierwotny programista nie wprowadził problemów celowo, a czasem mógł pomyśleć, że był wystarczająco dobry (a czasem nawet może). Ale ta druga para oczu daje zupełnie nowy widok i może się wiele nauczyć od patrzenia na problemy.
To naprawdę jest kwestia kulturowa i wymaga dużo zaufania i komunikacji. I czas na pracę z wynikami.
źródło
Myślę, że pozytywną rzeczą byłoby uzyskanie konsensusu w sprawie standardów kodowania przed dokonaniem recenzji. Inni mają tendencję do kupowania czegoś więcej, jeśli mają wkład.
W przypadku konwencji nazewnictwa zapytaj innych, czy jest to ważne. Większość programistów zgodzi się zwłaszcza wśród swoich rówieśników. Kto chce być osobą, która nie chce się zgodzić z czymś tak powszechnym w świecie programowania? Kiedy zaczynasz proces od wybrania czyjegoś kodu i wskazania wady, stają się bardzo defensywni. Kiedy zostaną ustalone standardy, nie będzie zgody co do interpretacji lub tego, co uważa się za „wystarczająco dobre”, ale jest lepiej niż teraz.
Inną częścią tego procesu jest ustalenie, w jaki sposób recenzje kodu będą obsługiwać zastrzeżenia. To nie może stać się niekończącą się debatą. W pewnym momencie ktoś musi podjąć decyzję. Może może być osoba trzecia, która będzie sędzią, lub recenzent przejmie całą moc. Cel musi być załatwiony.
Ostatnim krokiem jest poinformowanie wszystkich, że recenzje kodu nie były twoim pomysłem, pozostaną, więc każdy powinien dołożyć starań, aby jak najlepiej to wykorzystać. Jeśli wszyscy czują się bezsilni, być może mogą mieć możliwość sprawdzenia Twojego kodu?
Mamy nadzieję, że wymiernym rezultatem dla kierownictwa jest ograniczenie błędów, skarg klientów, opóźnień itp. W przeciwnym razie ktoś właśnie usłyszał modne hasło „przegląd kodu” i stwierdził, że jeśli doda je do procesu, cuda wystąpią bez dużo czasu, energii i włożony w to wysiłek.
źródło
Może to być trudne, ale nie martw się o dobre opinie, jeśli nie ma nic dobrego do zmierzenia.
Jednak w przyszłości, gdy programiści zaczną ulepszać swój kod, wtedy będziesz chciał przekazać im dobre opinie. Będziesz chciał wskazać ulepszenia w kodzie, a także wskazać korzyści dla całego zespołu. Kiedy zaczniesz dostrzegać poprawę, one również będą i rzeczy powinny powoli zacząć się pojawiać.
Inna rzecz; może być powietrze obronne, ponieważ czują się tak, jakby nie mieli nic do powiedzenia. Dlaczego nie pozwolić sobie na wzajemny przegląd kodu? Zadaj im konkretne pytania i spróbuj je zaangażować. Przeciwnie, nie powinieneś być ty; powinien to być wysiłek zespołu.
Zapytaj o to teraz i zapytaj o to za sześć miesięcy. Tutaj jest doświadczenie uczenia się.
Główny punkt - nie wychwalaj kodu, jeśli nie jest to uzasadnione, ale rozpoznaj wysiłek i zdecydowanie ulepszenie. Staraj się, aby recenzje były ćwiczeniami zespołowymi, a nie przeciwnymi.
źródło
Jakość bez napięcia
Pytałeś, jak znaleźć pozytywne rzeczy do powiedzenia na temat kodu, ale Twoim prawdziwym pytaniem jest, w jaki sposób możesz uniknąć „napięć w [twoim] zespole”, rozwiązując jednocześnie „poważne problemy z jakością”.
Stara sztuczka polegająca na umieszczaniu „złych wiadomości w dobrych wiadomościach” może przynieść odwrót. Deweloperzy prawdopodobnie zobaczą to, co to jest: pomysł.
Problemy odgórne w Twojej organizacji
Twoi szefowie powierzyli ci zadanie zapewnienia jakości. Wymyśliłeś listę kryteriów jakości kodu. Teraz potrzebujesz pomysłów na pozytywne wzmocnienie dla swojego zespołu.
Po co pytać nas, co musisz zrobić, aby Twój zespół był szczęśliwy? Czy zastanawiałeś się nad zapytaniem swojego zespołu?
Wygląda na to, że starasz się być miły. Problemem nie jest sposób dostarczania wiadomości. Problem polega na tym, że komunikacja była jednokierunkowa.
Budowanie kultury jakości
Kultura jakości musi być pielęgnowana, aby rosła od podstaw.
Poinformuj swojego szefa, że obawiasz się, że jakość nie poprawia się wystarczająco szybko i chcesz spróbować skorzystać z porad z Harvard Business Review .
Spotkaj się ze swoim zespołem. Modeluj zachowania, które chcesz zobaczyć w swoim zespole: pokora, szacunek i zaangażowanie na rzecz poprawy.
Powiedz coś w stylu: „Jak wiesz, [współpracownik] i ja mieliśmy za zadanie zapewnić jakość kodu, aby uniknąć problemów z produkcją, które ostatnio napotkaliśmy. Osobiście nie sądzę, że rozwiązałem ten problem. Myślę, że moim największym błędem nie było zaangażowanie was wszystkich na początku. [współpracownik] i ja nadal jesteśmy odpowiedzialni przed kierownictwem za jakość kodu, ale idąc dalej, wszyscy wspólnie pracujemy nad jakością. ”
Uzyskaj pomysły od zespołu na temat jakości kodu. (Tablica pomogłaby.) Upewnij się, że twoje wymagania dotrą tam do końca. Przekaż swoje spostrzeżenia z szacunkiem i zadawaj pytania w razie potrzeby. Bądź zaskoczony tym, co Twój zespół uważa za ważne. Bądź elastyczny, bez uszczerbku dla potrzeb biznesowych.
(Jeśli masz jakiś stary kod, który Cię zawstydza, możesz go wypróbować, zachęcając wszystkich do bycia szczerym. Na koniec daj znać ludziom, że go napisałeś. Modeluj dojrzałą reakcję, na którą liczysz, gdy inni otrzymają krytykę konstrukcyjną. )
Recenzje kodów współpracy
Nie pracowałem w miejscu, które opisujesz, gdzie kilku starszych programistów sprawdza cały kod i wprowadza poprawki. Nic dziwnego, że ludzie odpowiadają, jakbyś był nauczycielem, robiąc czerwone ślady na swoich papierach.
Jeśli możesz sprzedać pomysł zarządzaniu, zacznij robić recenzje kodów równorzędnych . Najlepiej zrobić to w małych grupach, w tym ty lub inny odpowiedzialny programista. Upewnij się, że wszyscy są traktowani z szacunkiem.
Ważnym celem recenzowania kodu jest upewnienie się, że kod jest zrozumiały dla wszystkich członków zespołu. Rozważ swój przykład niejasnych nazw funkcji: usłyszenie od młodszego programisty, że uważają, że nazwa funkcji jest myląca, może być łatwiejsze do zaakceptowania niż inna „korekta” od starszego programisty.
Jakość to podróż
Inną rzeczą do zrobienia jest obalenie pojęcia, że przegląd kodu jest czymś pozytywnym / negatywnym. Każdy powinien spodziewać się wprowadzenia pewnych zmian po przeglądzie kodu. (Twoim zadaniem jest dopilnowanie, aby się wydarzyły)
Ale jeśli to nie zadziała ...
Załóżmy, że poczyniłeś pewne kroki w celu ustanowienia kultury jakości. Nadal możesz nie wziąć wszystkich na pokład. Jeśli ktoś nadal nie jest objęty programem jakości, teraz problem polega na tym, że nie pasuje on do zespołu, a nie między wami jest problem. Jeśli będą musieli zostać odwołani z zespołu, reszta zespołu lepiej zrozumie powody.
źródło
Przepraszam za kolejną długą odpowiedź, ale nie sądzę, aby inni w pełni zajęli się ludzkim elementem tego problemu.
Problem z „Dlaczego wdrożyłeś go w ten sposób?” polega na tym, że natychmiast stawiasz programistę w defensywie. Samo pytanie może sugerować różne rzeczy w zależności od kontekstu: czy jesteś zbyt głupi, aby wymyślić lepsze rozwiązanie? Czy to najlepsze, co możesz zrobić? Próbujesz zrujnować ten projekt? Czy dbasz nawet o jakość swojej pracy? itd. Pytanie „dlaczego” kod został opracowany w określony sposób, będzie tylko konfrontacyjne, a to obali wszelkie pedagogiczne zamiary, jakie mógł mieć twój komentarz.
Podobnie „zrobiłbym to inaczej ...” jest również konfrontacyjne, ponieważ bezpośrednia myśl dewelopera będzie brzmiała: „ Cóż, zrobiłem to w ten sposób ... Masz z tym problem? ” I znowu, jest bardziej konfrontacyjny niż tak musi być i odwraca dyskusję od ulepszania kodu.
Przykład:
Zamiast pytać „Dlaczego nie użyłeś zmiennej stałej dla tej wartości?”, Po prostu powiedz „Ta zakodowana wartość powinna zostać zastąpiona stałą
XYZ
w plikuConstants.h
”. Zadanie pytania sugeruje, że programista aktywnie postanowił nie używać zdefiniowana stała, ale jest całkiem możliwe, że nawet nie wiedzieli, że istnieje. Mając wystarczająco dużą bazę kodu, będzie wiele rzeczy, o których każdy programista nie wie. Jest to po prostu dobra okazja do nauki dla tego programisty w celu zapoznania się ze stałymi projektu.Z recenzjami kodu jest cienka linia: nie musisz nakładać cukru na wszystko, co mówisz, nie musisz umieszczać złych wiadomości z dobrymi wiadomościami i nie musisz łagodzić ciosu. Może to być kultura, w której się znajduję, ale moi współpracownicy i ja nigdy nie komplementujemy się w recenzjach kodu - części kodu, które tworzymy, które są wolne od wad, są wystarczającym komplementem. Musisz zidentyfikować wadę, którą widzisz, i być może podać powód (dlaczego jest mniej przydatny, gdy jest oczywisty / prosty).
Dobry:
„Nazwę tej zmiennej należy zmienić, aby pasowała do naszego standardu kodowania”.
„Litera„ b ”w nazwie tej funkcji musi być pisana wielkimi literami, aby być PascalCase.”
„Kod tej funkcji nie jest prawidłowo wcięty”.
„Ten kod jest duplikatem kodu znalezionego w
ABC::XYZ()
i powinien zamiast tego użyć tej funkcji.”„Powinieneś używać try-with-resources, aby zagwarantować prawidłowe zamknięcie tej funkcji, nawet w przypadku wystąpienia błędów.” [Dodałem tylko link tutaj, aby użytkownicy spoza Javy wiedzieli, co oznacza try-with-resources]
„Tę funkcję należy zmienić, aby spełnić nasze standardy złożoności n-ścieżki”.
Zły:
„Myślę, że możesz ulepszyć ten kod, zmieniając nazwę zmiennej, aby pasowała do naszego standardu”
„ Być może lepiej byłoby użyć try-with-resource, aby poprawnie zamknąć rzeczy w tej funkcji”
„ Może być pożądane ponowne przyjrzenie się wszystkim warunkom tej funkcji. Złożoność ścieżki N przekracza maksymalną dozwoloną złożoność naszego standardu”.
„Dlaczego wcięcia są tutaj 2 spacjami zamiast 4 naszych standardów?”
„Dlaczego napisałeś funkcję, która łamie nasz standard złożoności n-ścieżki?”
W złych stwierdzeniach części pisane kursywą to rzeczy, których ludzie często używają, gdy chcą złagodzić cios, ale wszystko, co naprawdę robi w przeglądzie kodu, sprawia, że wydajesz się niepewny. Jeśli ty, recenzent, nie masz pewności, jak poprawić kod, to dlaczego ktoś miałby cię słuchać?
„Dobre” stwierdzenia są tępe, ale nie oskarżają programisty, nie atakują programisty, nie są konfrontacyjne i nie kwestionują przyczyny wady. Istnieje; oto poprawka. Z pewnością nie są tak konfrontacyjne jak to ostatnie pytanie „dlaczego”.
źródło
Problem, który widzisz, jest taki: programiści są niezadowoleni, że ich kod jest krytykowany. Ale to nie jest problem. Problem polega na tym, że ich kod nie jest dobry (zakładając oczywiście, że twoje recenzje kodu są dobre).
Jeśli programiści nie lubią krytyki kodu, rozwiązanie jest proste: Napisz lepszy kod. Mówisz, że miałeś poważne problemy z jakością kodu; oznacza to, że potrzebny jest lepszy kod.
„Dlaczego metoda ma mieć sensowną nazwę, wiem, co robi?” jest to coś, co szczególnie mnie niepokoi. On wie, co robi, a przynajmniej tak mówi, ale ja nie. Każda metoda nie powinna mieć po prostu rozsądnej nazwy, powinna mieć nazwę, która natychmiast wyjaśni czytelnikowi kodu, co robi. Możesz odwiedzić stronę Apple i poszukać filmu WWDC na temat zmian z Swift 2 na Swift 3 - ogromna liczba zmian wprowadzonych tylko po to, aby wszystko było bardziej czytelne. Być może tego rodzaju wideo może przekonać programistów, że programiści, którzy są o wiele mądrzejsi, uważają intuicyjne nazwy metod za bardzo, bardzo ważne.
Kolejnym niepokojącym elementem był twój kolega, który powiedział: „powiedziała mi sama, że po zgłoszeniu błędu przez klienta, wiedziała o błędzie, ale obawiała się, że deweloper byłby na nią zły za wskazanie go”. Istnieje możliwość, że istnieją pewne nieporozumienie, ale jeśli deweloper jest zły, jeśli zwrócić uwagę na błąd nich, że jest zły.
źródło
Z szacunkiem nie zgadzam się z opisaną przez ciebie metodą przeglądu kodu. Dwaj pracownicy wychodzący do „zamkniętego pokoju” i wychodzący z krytyką instytucjonalizują ten rodzaj kontrowersyjnej koncepcji, której należy unikać dobrych przeglądów kodu .
Sprawienie, by przegląd kodu był pozytywnym doświadczeniem w możliwie najszerszym zakresie, jest niezbędny do jego sukcesu. Pierwszym krokiem jest usunięcie przeciwnego pojęcia przeglądu. Zrób z tego cotygodniowe lub dwutygodniowe wydarzenie grupowe ; upewnij się, że wszyscy wiedzą, że są mile widziani. Zamów pizzę, kanapki lub cokolwiek innego. Nawet nie nazywaj tego „przeglądem kodu”, jeśli wywołuje to negatywne skojarzenia. Znajdź coś do świętowania, zachęć, podziel się - nawet jeśli jest to nic innego jak postęp w bieżącym sprincie lub iteracji. Na zmianę przydzielaj przywództwo nad przeglądem.
Spraw, aby proces ten był staraniem służenia produktowi i ludziom. Jeśli są one wykonywane w sposób konstruktywny, pozytywny, w którym dzielone są dobre techniki lub wzorce i zachęcane tak samo, jak zniechęcane są te słabe - wszyscy odnoszą korzyści. Wszyscy są szkoleni publicznie. Jeśli masz programistę, który nie „rozumie”, należy się tym zająć prywatnie i osobno - nigdy przed szerszą grupą. To jest niezależne od recenzji kodu.
Jeśli masz problem ze znalezieniem czegoś „dobrego” do poruszenia, to rodzi się pytanie: jeśli w projekcie poczyniono postępy, a ludzie pracują, ten postęp sam w sobie jest czymś do świętowania. Jeśli naprawdę nie znajdujesz nic dobrego, oznacza to wszystko, co zostało zrobione, ponieważ ostatnia recenzja jest albo zła, albo nie lepsza niż neutralna . Czy to naprawdę tak jest?
Jeśli chodzi o szczegóły, niezbędne są wspólne standardy. Daj wszystkim udział w tym, co się dzieje. I pozwól, aby nowsze, lepsze techniki zostały zintegrowane z bazą kodu. Nieprzestrzeganie tego gwarantuje, że stare nawyki nigdy nie zostaną wyeliminowane pod pozorem „zawsze tak robiliśmy”.
Chodzi o to, że recenzje kodu są źle traktowane, ponieważ są słabo zaimplementowane, używane jako młoty do umniejszania mniej doświadczonych lub mniej wykwalifikowanych programistów, i to nie jest przydatne dla nikogo. Menedżerowie, którzy używają ich w ten sposób, raczej nie będą bardzo skutecznymi menedżerami. Dobre recenzje kodu, w których każdy jest uczestnikiem, służą wszystkim - produktowi, pracownikom i firmie.
Przepraszam za długość postu, ale będąc w grupie, w której przegląd kodu został w dużej mierze zignorowany, doprowadziło to do spuścizny nieusuwalnego kodu i tylko wąskiego zakresu programistów, którzy byliby w stanie, nawet jeśli pozwolono wprowadzić zmiany w bazie kodu od lat. To była ścieżka, której wolałbym nie pokonywać ponownie.
źródło
Paradoks dobrego kodu polega na tym, że nie wyróżnia się on wcale i wygląda na to, że był bardzo prosty i łatwy do napisania. Bardzo podoba mi się przykład gracza w bilard z tej odpowiedzi . W recenzjach kodu sprawia to, że bardzo łatwo jest go przeświecać, chyba że zdarza się, że jest to refaktor przed złym kodem.
Poszukuję tego. Jeśli przeglądam kod i przejrzałem plik, który był tak łatwy do odczytania, że jego pisanie wydaje się zwodniczo łatwe, po prostu wrzucam szybkie „Podoba mi się, że metody tutaj są krótkie i czyste” lub cokolwiek jest odpowiednie .
Powinieneś także dawać przykład. Powinieneś nalegać, aby Twój kod został również przejrzany i powinieneś modelować, jak chcesz, aby twój zespół zachowywał się po otrzymaniu poprawki. Co najważniejsze, należy szczerze podziękować ludziom za opinie. To robi większą różnicę niż jakakolwiek jednostronna informacja zwrotna, którą możesz udzielić.
źródło
Wygląda na to, że prawdziwym problemem jest to, że tylko dwie osoby wykonują wszystkie przeglądy kodu, z których to tylko Ty traktujesz je poważnie, co doprowadziło cię do niefortunnej sytuacji z dużą odpowiedzialnością i dużą ilością presja ze strony innych członków zespołu.
Lepszym sposobem byłoby rozłożenie odpowiedzialności za dokonywanie przeglądów kodu na zespół i umożliwienie wszystkim udziału w ich wykonywaniu, na przykład poprzez umożliwienie programistom wyboru, komu mają przejrzeć swój kod. Jest to coś, co powinno obsługiwać narzędzie do sprawdzania kodu.
źródło
Widziałem to z pierwszej ręki i chcę odciągnąć cię od odpowiedzi kwestionowanych przez inteligencję emocjonalną - mogą zabijać całe zespoły. O ile nie chcesz rekrutować, szkolić i normalizować nowych programistów każdego roku, tworzenie pozytywnych relacji z rówieśnikami jest niezbędne. W końcu czyż nie ma sensu robić tych recenzji w celu poprawy jakości kodu i wspierania kultury, w której jakość kodu jest przede wszystkim wyższa? Twierdziłbym, że rzeczywiście częścią twojego zadania jest zapewnienie pozytywnego wzmocnienia jako sposobu na zintegrowanie tego systemu przeglądu kodu z kulturą zespołu.
W każdym razie wygląda na to, że musisz przejrzeć system Code Review. W tej chwili z brzmienia wynika, że wszystko w procesie recenzji jest lub może być interpretowane jako subiektywne, a nie obiektywne. Łatwo jest zranić uczucia, gdy wydaje się, że ktoś po prostu rozbiera twój kod, ponieważ go nie lubi, zamiast mieć powód, dla którego może cytować, gdy coś nie pasuje do wytycznych. W ten sposób można łatwo śledzić i „świętować” pozytywną poprawę jakości kodu (w odniesieniu do systemu recenzji) w sposób odpowiedni dla kultury biura.
Koordynatorzy techniczni muszą zebrać się poza sesją przeglądową i wydać wytyczne dotyczące kodowania / listę kontrolną przeglądu kodu, do których należy się stosować i do których należy się odwoływać podczas przeglądu. Powinien to być żywy dokument, który można aktualizować i udoskonalać w miarę rozwoju procesu. Spotkania liderów technicznych powinny odbywać się również wtedy, gdy powinna się odbyć dyskusja „Sposób, w jaki zawsze coś robiliśmy” vs. Gdy wstępna wytyczna zostanie mniej lub bardziej wygładzona, innym sposobem pozytywnego wzmocnienia deweloperów jest poproszenie o opinie, a następnie podjęcie działań w tej sprawie i ewoluuj proces jako zespół, to cuda, aby przyśpieszyć je, aby zacząć sprawdzać kod na pokładzie, więc nie utkniesz w robieniu recenzji, dopóki nie przejdziesz na emeryturę.
źródło
Lokal...
Programiści rozwiązują problemy. Jesteśmy uwarunkowani, aby rozpocząć debugowanie, gdy pojawia się problem lub błąd.
Kod jest medium formatu konturu. Przejście do narracji w formacie akapitu w celu uzyskania informacji zwrotnej wprowadza rozłączenie, które musi zostać przetłumaczone. Nieuchronnie coś zgubi się lub źle zrozumie. Nieuniknione jest, że recenzent nie mówi w języku programisty.
Wygenerowane komputerowo komunikaty o błędach rzadko są kwestionowane i nigdy nie są traktowane jako osobisty afront.
Elementy przeglądu kodu to komunikaty o błędach generowane przez ludzi. Mają na celu uchwycić przypadkowe przypadki, których nie zauważają programiści i zautomatyzowane narzędzia, a także nieuniknione skutki uboczne innych programów i danych.
Wnioski ...
Im więcej elementów do przeglądu kodu można włączyć do zautomatyzowanych narzędzi, tym lepiej będą one otrzymywane.
Zastrzeżenie polega na tym, że aby pozostać niekwestionowanym, takie narzędzia muszą być stale aktualizowane, zwykle codziennie lub co tydzień, aby były zgodne z każdą zmianą procedur, standardów i praktyk. Kiedy menedżer lub zespół programistów decyduje się na zmianę, narzędzia muszą zostać zmienione, aby to wymusić. Znaczna część pracy recenzenta kodu staje się wtedy utrzymywaniem reguł w narzędziach.
Przykład opinii dotyczącej przeglądu kodu ...
Przepisać podany przykład, uwzględniając następujące techniki:
źródło
Jeśli nie masz nic miłego do powiedzenia na temat istniejącego kodu (co wydaje się zrozumiałe), spróbuj zaangażować programistę w jego ulepszanie.
W łatce na funkcjonalną zmianę lub poprawkę błędu nie jest pomocne, aby inne zmiany (zmiana nazwy, refaktoryzacja, cokolwiek) były dołączone do tej samej poprawki. Powinien dokonać zmiany, która naprawia błąd lub dodaje funkcję, nic więcej.
Gdzie zmiana nazwy, refaktoryzacja i inne zmiany są wskazane, należy je w osobnej poprawki, przed lub po.
Pomaga również, jeśli dołączasz do refaktoryzacji i pozwalasz im przejrzeć Twój kod. Daje ci to szansę, by powiedzieć, dlaczego uważasz, że coś jest dobre, a nie złe, a także pokazać przykład dobrze przyjmującego konstruktywną krytykę.
Jeśli chcesz dodać testy jednostkowe do nowej funkcji, dobrze, idą w głównej łatce. Ale jeśli naprawiasz błąd, testy powinny przejść do poprzedniej łatki, a nie przejść pomyślnie, abyś mógł wykazać, że poprawka naprawiła błąd.
Najlepiej byłoby przedyskutować plan (w jaki sposób przetestować poprawkę lub co zrefaktować i kiedy) przed wykonaniem pracy, aby nie włożyli w to zbyt wiele wysiłku, zanim odkryją, że masz z tym problem.
Jeśli okaże się, że kod nie radzi sobie z danymi wejściowymi, które Twoim zdaniem powinny, powinny być również niezgodne z tym, co programista uważa za rozsądne dane wejściowe. Zgadzam się też z góry, jeśli to możliwe. Przynajmniej porozmawiaj o tym, z czym powinien sobie poradzić i jak okropnie może się nie udać.
źródło
Myślę, że błędem jest założenie, że istnieje techniczne lub łatwe rozwiązanie: „moi współpracownicy są zdenerwowani, gdy oceniam ich pracę według moich standardów i mam moc, by ją egzekwować”.
Pamiętaj, że recenzje kodu to nie tylko dyskusja o tym, co dobre lub złe. Są domyślnie „kimś, kogo używamy”, „który podejmuje decyzję”, a zatem - prawie podstępnie - rangą.
Jeśli nie rozwiążesz tych kwestii, wykonanie recenzji kodu w sposób, który nie napotka problemów, będzie trudne. Jest tu kilka dobrych rad, jak to zrobić, ale postawiłeś sobie trudne zadanie.
Jeśli masz rację, bez żadnego pokoju poruszania się, wskazywanie na to jest atakiem: ktoś się pomylił. Jeśli nie: inny MBA, który go nie otrzymuje. Tak czy inaczej, będziesz złym facetem.
Jeśli jednak to, czego szuka przegląd i dlaczego , jest jasne i uzgodnione, masz przyzwoitą szansę, aby nie być złym facetem. Nie jesteś strażnikiem, tylko korektorem. Uzyskanie tej umowy jest trudnym problemem do rozwiązania [1]. Chciałbym dać ci radę, ale ja sam jeszcze jej nie zrozumiałem ...
[1] Nie rozwiązano tego tylko dlatego, że ludzie powiedzieli „ok” lub przestali się o to kłócić. Nikt nie chce być facetem, który twierdzi, że X po prostu nie jest praktyczny dla naszej {inteligencji, doświadczenia, siły ludzkiej, terminów itp.}, Ale to nie znaczy, że w rzeczywistości chodzi o konkretny przypadek robienia X ...
źródło
Przeglądy kodu powinny być wzajemne. Wszyscy krytykowali wszystkich. Niech motto brzmi: „Nie złość się, wyrównaj”. Wszyscy popełniają błędy, a gdy ludzie powiedzą, jak naprawić swój kod, zrozumieją, że to zwykła procedura, a nie atak na nich.
Widzenie kodu innych osób jest pouczające. Zrozumienie kodu do tego stopnia, że możesz wskazać jego słaby punkt i musisz wyjaśnić, że słabość może cię wiele nauczyć !
W recenzji kodu nie ma miejsca na pochwały, ponieważ chodzi o to, aby znaleźć wady. Możesz jednak pochwalić poprawki . Można chwalić zarówno aktualność, jak i schludność.
źródło