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
X
został przekazany jako argument do metodyY
? - 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”?
Odpowiedzi:
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ś:
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”.
źródło
bigger
, gdzie nie rozwiązuje się problemów. Takich jak brakujące indeksy w tabeli DB. Lub próba użycia metodologii lub algorytmu bez zrozumienia uzasadnienia i jako taka niepoprawna. Dla mnie - są one ważniejsze i należy je rozwiązać i rozwiązać przede wszystkim - a estetyka stanowi drugorzędną kwestię.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ć.
źródło
ocean of shit
napisane przeze mnie - zachęcam do zasugerowania przepisania. Ale jeśli zignorujesz,shit
ale poprosisz mnie o naprawienie kosmetycznych rzeczy - to mnie frustruje.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 .
źródło
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:
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.
źródło
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 .
źródło
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”):
„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)):
true
, to trochę paranoiczne ...”, ...)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):
źródło
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.
źródło