Co zawiera standardowa recenzja kodu?

19

W mojej firmie jest to dyskusja e-mailowa o tym, która funkcja jest zaimplementowana i jaki rodzaj błędu został naprawiony, wysłany przez tego, kto pisze kod. A recenzent, który otrzyma pocztę, przejrzy kod i omówi jego jakość oraz sposób jego edycji. Co zawiera standardowa recenzja kodu?

Nicole
źródło
10
Tutaj najwyraźniej nie mamy czasu na recenzje kodu, ale mnóstwo czasu na poradzenie sobie z wynikającymi z tego błędami. Chciałbym żartować.
MetalMikester

Odpowiedzi:

12

Z mojego doświadczenia wynika, że ​​większość formalnych recenzji kodu przekształca się w sprawdzanie stylu, ponieważ jest to łatwe. Nawet jeśli dostarczysz listę kontrolną rzeczy, na które można spojrzeć, oczy są całkiem łatwe do przeszklenia.

Odkryłem, że przegląd testów jednostkowych zapewnia więcej korzyści. Większość programistów, z którymi pracowałem, tak naprawdę nie wie, jak poprawnie testować jednostki, a kiedy dostaną „Aha!” w tej chwili reszta kodu również zaczyna się poprawiać. Oto podpowiedź: to nie jest test jednostki, jeśli wymagać od użytkownika, aby sprawdzić coś, i to nie jest test jednostki, jeśli dopiero zaczynasz coś uruchomić w debuggera.

Berin Loritsch
źródło
LOL, dobra znajomość testów jednostkowych jest koniecznością. Dobrą wiadomością jest to, że testowanie jest po prostu zdrowym rozsądkiem - zajmuje mniej czasu, aby dowiedzieć się, niż powiedzieć ... czas potrzebny na wybranie nowego języka.
Praca
Znalazłem się nitpicking na rzeczy, gdy brak jest pokrycia badanej jednostki. Kiedy widzę testy jednostkowe w swojej recenzji kodu, to pierwsze miejsce patrzę. Jeśli widzę, że testy jednostkowe są trafienia wymagań biznesowych i sensowne przypadki krawędź (czek na wartości null w stosownych przypadkach, badania na granicy zakresów wartości) następnie staram się nie nit pick --- co nie znaczy, że należy odebrać w małych rzeczach , To jest po prostu, że „dowód jest w pudding.” Trudno dyskutować z dobrze skonstruowanych testów jednostkowych, które są przechodzących.
Greg Burghardt
6

To skłania do zmiany oparte na czym polega problem. Wiele razy jest to prosta gumy pieczęć. „Oto, na czym polegał problem, spójrz na tę linię tutaj, oczywiste jest, co się dzieje nie tak, i tutaj to rozwiązałem”. „Tak, to dość oczywiste. Śmiało i sprawdzić go w.”

Ale kiedy coś bardziej zaangażować się dzieje, to zazwyczaj wygląda tak:

  • Uruchom program Check za modyfikacje na TortoiseSVN i uzyskać listę zmienionych plików.
  • Doprowadzić recenzentem do swojego biura.
  • Wyjaśnić problem, z CR z systemu śledzenia błędów otwartych w celach informacyjnych.
  • Przewiń listę plików w TortoiseSVN, otwierając każdy z nich w BeyondCompare, aby zobaczyć zmiany.
  • Jeśli recenzent nie rozumie zmian, wyjaśnić, co zrobił i dlaczego.
  • Recenzent może zauważyć coś, co nie wygląda dobrze. Jeśli tak, to omówić, aż dojdziesz do porozumienia w sprawie tego, czy nie powinno być zmieniane. (Jeśli konieczne jest wprowadzenie prostych zmian, można nawet edytować wewnątrz pliku BeyondCompare).
  • Jeśli zostały wprowadzone zmiany, przebudować i upewnić się, że buduje!
  • Uruchom program, aby pokazać recenzentowi, że poprawka faktycznie działa.
  • Sprawdź to.
Mason Wheeler
źródło
4

IMO, Przegląd kodu nie ma nic wspólnego z funkcjami ani błędami, ale koncentruje się na jakości kodu i napisanych dla niego testów.

Siedzisz więc obok rówieśnika i każesz mu wyjaśnić kod, lub wziąć kod i przejrzeć go, bez względu na sytuację.

To nie pomaga, gdy programy wszyscy przeciwko tych samych standardów i jeśli wykorzystują narzędzi, takich jak FxCop zautomatyzować część procesu.

syg
źródło
2

Wolę recenzję kodu, gdzie deweloper siedzi z recenzentem i przechodzi przez wiersz kodu po wierszu, wyjaśniając to. Często deweloper widzi problem z wyjaśnieniami, których recenzent jeszcze nie widział, dlatego właśnie to jest moje preferencje. Ja też zrobić opinie kodu, w którym mam wysłał kod i czytać na własnych i uwag, ale uważam, że te wydają się trwać dłużej (I przegląd i projekty komentarze i wysłać do dev kto czyta je i idzie WTF ona oznacza i e-maile mnie z powrotem i wyjaśnić i dwie lub trzy rundy później dostajemy razem i wskazać na ekranie, co mam na myśli i dev idzie, „oh yeah teraz to widzę.”) i będzie mniej wydajne, jak tam jest mniej prawdziwa dyskusja i więcej: „Zrobiłeś to źle”.

Jest to również kluczowe znaczenie dla egzekwowania standardów przeglądu kodu, ale nie do ich jedynym celem.

Jednak kod nie jest wysyłany do produkcji, dopóki recenzent kodu nie będzie zadowolony lub menedżer (nie programista) go unieważni (recenzenci kodu też się mylili). To jest krytyczny lub przegląd kodu jest tylko biurokratyczny proces bez wartości dodanej, chyba że kod Recenzent musi zatwierdzić ostateczny kod zanim zostanie wciśnięty.

HLGEM
źródło
Zawsze sugeruję, aby zależało od twórcy, co robi z opiniami z recenzji. Recenzent niekoniecznie wie najlepiej, a gdy umowa jest obowiązkowa, może być konieczne poświęcenie sporo czasu na edukację recenzenta. Zastanowiłbym się jednak nad ostateczną „integracją” przez starszego / wiodącego programistę.
Joppe
0

Najpierw musisz mieć standardy kodowania, które są czymś więcej niż tylko składnią. Kiedy ludzie zaczynają pracę w Twojej firmie, muszą jak najlepiej poznać wytyczne Twojej firmy, zanim zaczną kodować . Jeśli w trakcie procesu sprawdzania zostaną wykryte wszelkiego rodzaju naruszenia, najprawdopodobniej:

  • nie zostanie naprawiony z powodu ograniczeń czasowych
  • okazało się bardziej irytujące niż warte są wytyczne

Wytyczne powinny sensu i nie powinno być właściwe oprzyrządowanie znaleźć naruszeń i byłaby tak proste, jak to możliwe. Zawsze patrz na cel wytycznych oraz przeglądu kodu

Celem moim jest, aby pamiętać, kod jak najbardziej jednolite i znaleźć problemy z konserwacji i czytelność. Wtórny cel może być, aby uzyskać więcej ludzi do prędkości z pewnej części oprogramowania.

Wytyczne w moim umyśle może np istnieją od:

  • Ogólna składnia i wytyczne kodowania (wybrać ten, który już istnieje i użycia oprzyrządowania, który sprawdza automatycznie)
  • Właściwa obsługi wyjątków
  • Prawidłowe rejestrowanie
  • Dobre wykorzystanie paradygmatów dla języka (stała dla języków OO)
  • Oczywiste i dobrze przemyślane zależności między składnikami (używać narzędzi takich jak NDepend)
  • Robocza kompilacji skryptu
  • obecna dokumentacja (startup deweloper, instrukcja montażu)
  • wewnętrzne biblioteki do użytku
  • Polityka firmy
  • narzędzia innych firm, które są niedozwolone
  • Testy jednostkowe obecne i niezaliczone
  • pokrycie kodu 90%
  • ...

Mając to na miejscu przegląd Kod składa się z oprogramowania są sprawdzane pod kątem wytycznych i:

  • omówić naruszenia z programistą
  • naprawić niepotrzebne naruszenia
  • skomentuj niezbędne naruszenia
KeesDijk
źródło