Frustracje związane z recenzowaniem / kodowaniem

Nie nazwałbym siebie supergwiazdą, ale stosunkowo doświadczonym. Staram się utrzymywać jakość kodu na wysokim poziomie i zawsze staram się ulepszyć mój styl kodowania, staram się, aby kod był wydajny, czytelny i spójny, a także zachęcać zespół do przestrzegania wzorców i metodologii w celu zapewnienia spójności. Rozumiem także potrzebę równowagi między jakością i szybkością.

Aby to osiągnąć, przedstawiłem zespołowi koncepcję wzajemnej oceny. Dwa kciuki do góry w github-prośbie o połączenie. Świetnie - ale moim zdaniem nie bez czkawek.

Często widzę komentarze tych samych kolegów, takie jak -

• Dobrze byłoby dodać spację później <INSERT SOMETHING HERE>
• Niechciana dodatkowa linia między metodami
• Na końcu komentarzy w docblocks należy stosować kropkę.

Teraz z mojej perspektywy - recenzent powierzchownie patrzy na estetykę kodu - i tak naprawdę nie dokonuje przeglądu kodu. Przegląd kodu kosmetycznego przychodzi mi do głowy jako arogancka / elitarna mentalność. Brakuje mu treści, ale tak naprawdę nie można się z tym zbytnio kłócić, ponieważ recenzent jest technicznie poprawny . Wolałbym zobaczyć mniej z powyższych rodzajów recenzji, a więcej recenzji w następujący sposób:

• Możesz zmniejszyć złożoność cykliczną, ...
• Wyjdź wcześniej i unikaj if / else
• Wyodrębnij zapytanie DB do repozytorium
• Ta logika tak naprawdę tu nie pasuje
• Nie powtarzaj się - streszczenie i powtórne użycie
• Co by się stało, gdyby Xzostał przekazany jako argument do metody Y?
• Gdzie jest na to test jednostkowy?

Uważam, że zawsze są to ci sami ludzie, którzy oceniają kosmetyczne typy recenzji, i ci sami ludzie, którzy moim zdaniem wydają recenzje oparte na jakości i logice.

Jakie (jeśli w ogóle) jest prawidłowe podejście do wzajemnej oceny. I czy mam rację, gdy frustrują mnie te same osoby, które w zasadzie przeglądają kod, szukając błędów ortograficznych i defektów estetycznych zamiast faktycznych defektów kodu?

Jeśli mam rację - jak zachęcić kolegów do szukania błędów w kodzie w równowadze z sugerowaniem kosmetycznych poprawek?

Jeśli się mylę - proszę, oświeć mnie. Czy istnieją jakieś praktyczne zasady dotyczące tego, co stanowi dobrą recenzję kodu? Czy przegapiłem punkt, czym są recenzje kodu?

Z mojego punktu widzenia - przegląd kodu dotyczy wspólnej odpowiedzialności za kod. Nie czułbym się dobrze, podchodząc do kodu bez adresowania / sprawdzania logiki, czytelności i funkcjonalności. Nie zawracałbym sobie również głowy blokowaniem scalania solidnego fragmentu kodu, gdybym zauważył, że ktoś pominął kropkę w bloku doc.

Kiedy sprawdzam kod, spędzam może od 15 do 45 minut na 500 loc. Nie wyobrażam sobie, by te płytkie recenzje trwały dłużej niż 10 minut, jeśli taka jest głębokość recenzji, którą wykonują. Ponadto, jaką wartość ma kciuk od płytkiego recenzenta? Z pewnością oznacza to, że wszystkie kciuki nie są równej wadze i może być konieczny 2-etapowy proces przeglądu. Jeden kciuk do głębokich recenzji i drugi kciuk do „polerowania”?

Rodzaje recenzji

Nie ma jednego prawdziwego sposobu przeprowadzania recenzji. Istnieje wiele sposobów oceny, czy kod jest wystarczająco wysokiej jakości. Oczywiste jest pytanie, czy jest on wadliwy, czy ma rozwiązania, które nie skalują się lub są kruche. Kwestie zgodności z lokalnymi standardami i wytycznymi, choć być może nie tak krytyczne jak niektóre inne, są również częścią tego, co przyczynia się do wysokiej jakości kodu.

Rodzaje recenzentów

Tak jak mamy różne kryteria oceniania oprogramowania, tak i osoby oceniające są różne. Wszyscy mamy własne umiejętności i upodobania. Niektórzy mogą myśleć, że przestrzeganie lokalnych standardów jest bardzo ważne, podobnie jak inni mogą być bardziej zainteresowani wykorzystaniem pamięci lub pokryciem kodem twoich testów i tak dalej. Chcesz wszystkich tych rodzajów recenzji, ponieważ jako całość pomogą Ci napisać lepszy kod.

Recenzja to współpraca, a nie gra w tag

Nie jestem pewien, czy masz prawo powiedzieć im, jak mają wykonywać swoją pracę. O ile nie wiesz z całą pewnością inaczej, załóż, że ta osoba stara się przyczynić do tego, co uważa za stosowne. Jeśli jednak widzisz pole do poprawy lub podejrzewasz, że może nie rozumieją, czego oczekuje się od wzajemnej oceny, porozmawiaj z nimi .

Celem wzajemnej oceny jest zaangażowanie twoich rówieśników . Zaangażowanie nie polega na rzucaniu kodu przez ścianę i czekaniu na odpowiedź z powrotem. Zaangażowanie działa razem, aby ulepszyć kod. Weź udział w rozmowie z nimi.

Rada

Pod koniec pytania napisałeś:

jak miałbym zachęcać kolegów do szukania błędów w kodzie w równowadze z rażącymi błędami estetycznymi?

Ponownie odpowiedzią jest komunikacja. Być może możesz ich zapytać: „hej, doceniam to, że łapiesz te błędy. Bardzo by mi pomogło, gdybyś mógł skupić się również na niektórych głębszych kwestiach, takich jak to, czy właściwie tworzę swój kod. Wiem, że to wymaga czasu, ale to naprawdę Wsparcie."

Mówiąc bardziej pragmatycznie, osobiście dzielę komentarze recenzji kodu na dwa obozy i odpowiednio je wyrażam: rzeczy, które należy naprawić, i rzeczy, które są bardziej kosmetyczne. Nigdy nie zapobiegłbym sprawdzeniu stałego, działającego kodu, jeśli na końcu pliku było zbyt wiele pustych linii. Zwrócę jednak na to uwagę, ale zrobię to z czymś takim, jak „nasze wytyczne mówią, że na końcu jest jedna pusta linia, a ty masz 20. To nie jest przeszkoda, ale jeśli masz szansę, może chcieć to naprawić ".

Oto jeszcze jedna rzecz do rozważenia: może to być twój wkurzony zwierzak, że dokonują tak płytkiej weryfikacji twojego kodu. To może być bardzo dobrze, że mnie denerwuje z nich jest to, że (lub jakiś inny kolega, który dostaje podobną opinię) są niechlujstwa w odniesieniu do standardów kodowania własnej organizacji, a to, jak wybrali się komunikować, że z wami.

Co robić po przeglądzie

I na koniec trochę porad po przeglądzie: popełniając kod po przeglądzie, możesz rozważyć zadbanie o wszystkie kosmetyczne rzeczy w jednym zatwierdzeniu, a zmiany funkcjonalne w innym. Mieszanie tych dwóch może utrudnić odróżnienie znaczących zmian od nieznacznych. Wprowadź wszystkie zmiany kosmetyczne, a następnie zatwierdź za pomocą komunikatu „kosmetyczny; bez zmian funkcjonalnych”.

Ludzie komentują formatowanie kodu i literówki, ponieważ są łatwe do wykrycia i nie wymagają od nich dużego wysiłku.

Ta część jest łatwa do naprawienia - prawie każdy język ma narzędzie do sprawdzania linii lub stylu. Podłącz go do procesu kompilacji, aby nie powiódł się kompilacja, jeśli jest nadmiarowe miejsce. W rezultacie nie będzie żadnych problemów ze stylem do komentowania.

Doprowadzenie ich do znalezienia prawdziwych problemów może być sporym wyzwaniem. Wymaga to nie tylko dociekliwego i zorientowanego na szczegóły sposobu myślenia, ale także znacznej motywacji.

Ta motywacja pochodzi z doświadczenia. Doświadczenie w próbach pracy z kiepskim kodem napisanym przez poprzednich programistów. Starsi inżynierowie mają tego mnóstwo. Pływanie w oceanie gówna sprawia, że ​​nie chcesz już tam wracać.

Gdybym musiał wybrać jedną główną rzecz do sprawdzenia podczas przeglądania kodu, byłaby to łatwość konserwacji kodu. Przez cały czas programista sprawdzający ten kod powinien go zrozumieć i być gotowy do jego ulepszenia i modyfikacji. Jeśli nie ma pojęcia, co się tutaj dzieje, musi to natychmiast powiedzieć, a kod trzeba przepisać.

Oto praktyczna odpowiedź:

Scenariusz 1 : Jesteś starszym członkiem i kimś, kto może zdecydować o praktyce

Zwołaj spotkanie i określ zasady i wytyczne Code Review. Zaufaj mi, jasne wytyczne działają lepiej niż jakikolwiek system lub szkolenie „honorowe”. Wyjaśnij, że nie wolno w ogóle zgłaszać problemów z formatowaniem kodu. Niektórzy członkowie będą się sprzeciwiać. Posłuchaj ich, a następnie poproś, aby postępowali zgodnie z wytycznymi przez pierwsze kilka miesięcy jako eksperyment. Pokaż chęć zmiany, jeśli obecne wytyczne nie działają.

Scenariusz 2 : Nie jesteś kimś, kto decyduje o praktyce lub jesteś stosunkowo młodszym członkiem zespołu

Najlepszym rozwiązaniem jest rozwiązanie problemów z przeglądaniem kodu, dopóki nie uda się osiągnąć scenariusza 1 .

Prostą odpowiedzią, aby zapobiec „trywialnym” komentarzom do recenzji kodu, jest naleganie (ze względu na wydajność), że to recenzent powinien je naprawić. Jeśli więc recenzent stwierdzi, że (horror!) Przegapiłeś przystanek lub źle przeliterowałeś komentarz, powinien to naprawić i przejść dalej.

Z mojego doświadczenia wynika, że:

• Twoi recenzenci przestają komentować te względnie bezcelowe komentarze i przekazują je z powrotem do naprawy.
• naprawi je tylko większość recenzentów OCD, co oznacza, że ​​większość twoich recenzji przejdzie. ma to efekt domina polegający na tym, że deweloperzy muszą dokonywać bardziej merytorycznych recenzji, ci, którzy zgłaszają ciekawostki, ponieważ tak naprawdę nie sprawdzają kodu, będą musieli uzasadnić, dlaczego wszystkie ich recenzje przechodzą bez komentarza.

Tak czy inaczej, jest to korzyść. Koszt nieudanej recenzji jest wysoki, jeśli chodzi o zmuszenie programisty do zatrzymania tego, nad czym pracuje, i ponownego odwiedzenia jego kodu, a następnie do następnej recenzji. Jeśli recenzja wykryje prawdziwą jakość kodu lub problemy architektoniczne, ten koszt jest wart każdego grosza, ale zapłacenie tego kosztu za ciekawostki nie jest.

Przejrzyj proces recenzji

Oprócz recenzji kodu sugerowałbym regularne sprawdzanie Procesu recenzji. Z pewnością można się tutaj wiele nauczyć, np. Jak poprawnie oceniać kod.

Zwykle niektóre z nich są niedoświadczone i po prostu nie wiedzą, czego szukać. Potrzebują trochę wskazówek nie tylko w odniesieniu do swojego kodu, ale także w zakresie pomocnych recenzji kodu. Przekazywanie opinii na temat recenzji doprowadzi do procesu uczenia się, który (pełny) spowoduje lepsze recenzje i lepszy kod.

Następnie dobrym pomysłem może być (luźne) sformalizowanie zestawu wartości i kryteriów, tego, co organizacja lub zespół postrzega jako Dobry Kod ™ oraz jakich anty-wzorców należy unikać. Nie chodzi o ustawienie czegoś. konkretnie, ale od samego początku uzyskiwania powszechnego konsensusu co do jakości kodu .

Jako ktoś, kto był po obu stronach tego (recenzowanie kodu napisanego przez innych, a także recenzowanie kodu, który napisałem), powiedziałbym, że mam trzy kategorie, do których każda opinia dotyczy. Po czwarte, jest też zachwycający przypadek „wszystko jest dobrze”.

„Byłoby miło, ale to cię nie zablokuje” (jeśli wszystkie opinie mieszczą się w tej kategorii, mogę wysłać prośbę o scalenie z „będzie scalony w XX: XX, chyba że powiesz mi, że chcesz je naprawić” lub „jasne, śmiało, aby się zameldować, niezależnie od tego, który blok rzuciłby system został wyłączony”):

• Zapominanie kropek na końcu zdań (w ciągach dokumentów lub komentarzach). Niezdarna gramatyka we wszystkim, co nie jest wysyłane do użytkowników w jakikolwiek sposób w kształcie lub formie (ponownie, ciągi dokumentów i komentarze)
• Kod, który ma bardziej elegancki sposób, ale co jest zrozumiałe i idiomatyczne (prawdopodobnie wstawię nawet moją sugestię, więc łatwo jest wyjść lub naprawić)

„Rzeczy, które muszą zostać naprawione, ale zaufam ci, że to zrobisz” (jeśli wszystkie rzeczy mieszczą się w tej lub poprzedniej kategorii, odpowiem „napraw to, połączę się, gdy mi powiesz” ponownie zrobione ”(i w tym momencie prawdopodobnie szybko przejrzę, aby zobaczyć, że nic więcej nie wyskoczyło)):

• Drobne sprzeczki („porównujesz wartość logiczną true, to trochę paranoiczne ...”, ...)
• Drobne naruszenia stylu („przewodnik po stylu mówi X, to, co zrobiłeś, jest poza tym; zrobiłbym Y lub Z, w zależności od tego, w którą stronę chcesz iść”)
• Kilka brakujących przypadków testowych, które nie powinny być trudne do napisania

I wreszcie „rzeczy, które są problematyczne, po rozwiązaniu tych problemów będę musiał ponownie sprawdzić kod” (jeśli jest coś w tej kategorii, będzie trzeba przeprowadzić kolejną rundę recenzji, więc dodaj komentarz „ jest też kilka drobnych rzeczy, miło byłoby zobaczyć niektóre z nich naprawione, gdy jesteś przy tym „jeśli jest coś z dwóch pierwszych kategorii):

• Mogłoby to wyglądać tak: „nie, naprawdę istnieje O wiele lepszy sposób na napisanie tego”, „nie obliczasz tego, czego oczekujesz, ponieważ w teście jednostkowym nie ma takich przypadków skrajnych” i wszystkich innych poważnych problemów.

Wygląda na to, że niektórzy zapomnieli o najważniejszym pytaniu: co tak naprawdę chcesz osiągnąć dzięki recenzjom kodu?

Celem przeglądów kodu jest szybsze uzyskanie błędów i łatwego do utrzymania kodu. Przeglądy kodu osiągają to na kilka sposobów: programiści piszą przede wszystkim lepszy kod, ponieważ wiedzą, że zostanie sprawdzony, wiedza jest dzielona w ramach procesu przeglądu, zostaną znalezione błędy, ponieważ recenzent nie jest ślepy na własne błędy jako programiści może być.

Jeśli uważasz, że proces przeglądu jest szansą na odłożenie kolegów lub stworzenie dla nich pracy, oznacza to, że robisz to źle.