Jak ważne są pozytywne opinie w recenzjach kodu?

48

Czy ważne jest wskazanie dobrych części kodu podczas przeglądu kodu i powodów, dla których jest on dobry? Pozytywne opinie mogą być równie przydatne dla recenzowanego programisty, jak i dla innych osób biorących udział w recenzji.

Robimy recenzje za pomocą narzędzia online, więc programiści mogą otwierać recenzje pod kątem popełnionego przez nich kodu, a inni mogą sprawdzać swój kod w określonym czasie (np. 1 tydzień). Inni mogą komentować kod lub komentarze innych recenzentów.

Czy powinna istnieć równowaga między pozytywnym a negatywnym sprzężeniem zwrotnym?

c_maker
źródło
3
Hej, jeśli przejdzie, to pozytywne opinie. :)
Adrian J. Moreno,
1
Myślę, że w dużej mierze zależy to od osoby, której kod jest sprawdzany. Jeśli zareagują negatywnie na samą krytykę, ważne jest, aby znaleźć równowagę; w przeciwnym razie pozytywne opinie są zbędne, ponieważ pozytywna ocena jest pozytywna. Jeśli zrobią coś nowego i cudownego, możesz o tym wspomnieć, ale uwzględnienie tego w najlepszych praktykach Twojego zespołu również byłoby pozytywnym odzewem.
Matt
SE obejmuje zarówno opinie pozytywne, jak i negatywne, więc pozytywne opinie muszą być ważne, aby wszystko działało dobrze. Jak by to wyglądało, gdyby najlepsze, na co mogą liczyć twoje pytania i odpowiedzi, wynosi zero? Jest to stereotypowa różnica mężczyzn i kobiet: dla mężczyzn brak informacji zwrotnej oznacza „w porządku”. Dla kobiet oznacza to: „nie było nic dobrego do powiedzenia”. To może być bardzo dalekie od wyjaśnienia względnej atrakcyjności tego pola dla mężczyzn i kobiet.

Odpowiedzi:

41

Popraw jakość i morale za pomocą recenzji Peer Code
http://www.slideshare.net/SmartBear_Software/improve-quality-and-morale-using-peer-code-reviews

Rzeczy, które każdy powinien zrobić: Przegląd kodu
http://scientopia.org/blogs/goodmath/2011/07/06/things-everyone-should-do-code-review/

Oba te artykuły stwierdzają, że jednym z celów przeglądu kodu jest dzielenie się wiedzą na temat dobrych technik programowania, a nie tylko znajdowanie błędów.

Powiedziałbym, że to bardzo ważne. Kto chce iść na spotkanie i być tylko krytykowanym?

Robert Harvey
źródło
26
Mnie! Mnie! poza sarkazmem, naprawdę denerwuję się recenzjami kodu, gdzie nie ma krytyki mojego kodu; Wiem, że nie robiłem rzeczy idealnie, więc brak krytyki sprawia, że ​​mam wrażenie, że marnuję czas, nawet prosząc o recenzję. Zgadzam się więc, że tylko krytyka jest zła, ale żadna z nich nie jest też dobra.
Jimmy Hoffa,
3
Zwykle zgadzam się z Jimmy'm Hoffą. Ogólnie rzecz biorąc (nie tylko w recenzjach kodu) bardzo denerwuje mnie kontakt z ludźmi, którzy starają się uzyskać wiele pozytywnych opinii. Przydatne powinny być pozytywne opinie: nie trzeba zanieczyszczać recenzji, mówiąc rzeczy, które autor kodu już wie. Osobiście wolę podejście: „Jesteś świetny i mądry, ale w twoim kodzie jest kilka drobnych problemów”.
Arseni Mourzenko,
6
@MainMa: „Wygląda dobrze” działa dla mnie, jeśli nie znaleziono żadnych problemów. Dla szczególnie przydatnego lub dobrze napisanego kodu: „To może być przydatne. Umieśćmy je w naszym archiwum udostępniania kodu z uwagami lub spróbujmy włączyć to do naszej codziennej praktyki kodowania”.
Robert Harvey
2
Kiedyś ktoś zrezygnował z powodu tego, jak okropne były nasze recenzje kodu. Odtąd zmieniliśmy się na używanie recenzji kodu jako warsztatu, z odrobiną krytyki kodu za poważne problemy, ale głównie w celach edukacyjnych. Facet, który zrezygnował, wdał się w krzyczący mecz z naszym menedżerem podczas przeglądu z powodu różnic w opiniach. Ludzie mogą się naprawdę bronić podczas recenzji, więc gorąco polecam pozytywne opinie, aby złagodzić napięcie i ułatwić recenzentowi chwile „powinieneś to zmienić”, jeśli nie z innego powodu, jak od czasu do czasu głaszcząc ego
Brian,
2
@Brian Muszę powiedzieć, że jeśli ktoś wdaje się w krzyczący pojedynek z powodu niewielkiej krytyki, jest to prawdopodobnie czynnik zakłócający kulturę firmy i morale w biurze, myślę, że lepiej ci będzie.
Jimmy Hoffa,
29

Kiedy robię recenzje kodu, mam po prostu monolog, więc kiedy rozumiem, co czytam, będzie dużo „Ok, rozumiem, co to robi. Dobrze, że łączy się z tym i wywołuje że w porządku .. i ten kawałek zależy od obu tych elementów. ".

Myślę, że w ten sposób nie jest to „oo la la to takie wspaniałe!”, Może to być zupełnie trywialny nudny kod, ale słuchanie, jak ktoś naprawdę analizuje i pokazuje zrozumienie tego, co napisałeś, jest formą pozytywnej opinii samej w sobie, informacja zwrotna brzmi „Ten kod ma sens”, kiedy napotykam na części, których nie rozumiem, proszę o wyjaśnienie, a kiedy go rozumiem, wykrzyknę „Ach, rozumiem”.

Myślę, że proste przekazanie zrozumienia jest pochwałą dla innego inżyniera, ponieważ wszyscy chcemy, aby nasz kod był zrozumiały dla innych, daje on formę ukrytej weryfikacji.

To powiedziawszy, jeśli widzisz części kodu, które mają dobre lub pozytywne cechy (nawet nudny, trywialny kod może być dobry, jeśli jest to jego minimalna forma) zdecydowanie zdecydowanie stwierdzam te cechy, ponownie nie przypisuję ich jako „Wow świetny!" tyle, że „widzę, że jest to minimalna implementacja” lub „Ok, ten złożony algorytm ma wiele komentarzy”, skupiając się na atrybutach kodu, nie tyle na jego wrodzonej dobroci czy złości.

Za każdym razem, gdy przypisujesz „dobroć” lub „zło” kodowi w przeglądzie kodu, aby uniknąć sytuacji, w której inżynier czułby się zlekceważony lub trzymany na piedestale, nie mów, że coś jest dobre lub złe, ale raczej omów przyczynę i skutek ich kod.

„Ok, ta część ma sens, ah, tutaj jest magiczna liczba, znaczenie tej wartości może być niezrozumiałe dla następnego inżyniera, który by ją dotknął”

„Widzę, że masz tutaj pojemnik DI, więc będziesz mieć luźne połączenie z tym repozytorium”

„Ach, tutaj jest słownik statyczny, jeśli wiele wątków dotyka tego słownika, moglibyśmy napotkać warunki wyścigu”

Zauważ, że nie mówię, że coś jest dobre lub złe, ale to, czy inżynier powinien to zmienić, czy nie, zrozumie inżynier, którego kod jest sprawdzany. Oczywiście musisz zakończyć sprawdzanie kodu z yay lub nie, ale gromadzenie tych oświadczeń w ciągu tego okresu złagodzi słowa tak, ponieważ wyjaśnienia zostały już przedstawione w formie oświadczeń przyczynowo-skutkowych, kiedy im powiesz „Chciałbym te magiczne liczby ustalone przed sprawdzeniem tego w ”.

Jimmy Hoffa
źródło
4
+1 za „proste przekazanie zrozumienia jest pochwałą dla innego inżyniera ... daje formę ukrytej weryfikacji”
Roy Tinker,
Chcę dać +1 to dwa razy. Jeden z tego samego powodu, co @RoyTinker, a drugi dla „Nie mów, że coś jest dobre lub złe, ale mów o przyczynie i skutku”. Oba bardzo dobre punkty!
Ben Lee,
7

Gdybym zobaczył coś w recenzji kodu, który naprawdę mi się podobał i który wykraczał poza „wystarczająco dobry” kod, dałbym pozytywne opinie.

Ogólnie myślę, że jeśli ktoś napisze kod częściowy, który powoduje, że mówisz „Wow, to jest naprawdę miłe!” wtedy tak, pozytywna opinia jest ważna - informuje autora, że ​​to, co zrobili, podobało się innym i powinni spróbować to zrobić ponownie. Musi jednak być czymś więcej niż tylko przestrzeganiem wytycznych i standardowych praktyk. Wyrażanie pochwały, ponieważ ktoś ładnie wciął lub dodał komentarze na płycie głównej, może ustawić poprzeczkę raczej nisko.

FrustratedWithFormsDesigner
źródło
6

To nie jest tyle pytanie programistyczne, co ogólne pytanie dotyczące zarządzania i interakcji międzyludzkich. Pozytywne opinie w recenzjach kodu są tak samo ważne jak pozytywne opinie w jakiejkolwiek recenzji.

To, czy jest to wymagane (i zakres, w jakim jest to wymagane), jest funkcją usposobienia i emocjonalnego charakteru osoby, z którą rozmawiasz. Niektóre osoby reagują na korektę znacznie skuteczniej, gdy towarzyszy jej pochwała. Inni uważają pochwałę za nieszczerą, gdy jest dostarczana z poprawką.

Ogólna formuła jest czasami nazywana „Kanapką zwrotną”: najpierw dobre rzeczy, złe rzeczy drugie, dobre rzeczy ostatnie. Chodzi o to, aby utrzymać ogólny ton pozytywny, jednocześnie upewniając się, że otrzymano negatywne sprzężenie zwrotne. Może to pomóc w zapobieganiu stresowi w oczekiwaniu na przegląd, a także w zapobieganiu ponownemu wchłonięciu się lęków. Oba są bardzo ważne w odniesieniu do wydajności i jakości. To nie jest tylko emocjonalna miazga; To ludzkie zachowanie 101.

Ponownie musisz poznać osobę, z którą pracujesz i zrozumieć, na co ona reaguje. W kontaktach z ludźmi chodzi o zarządzanie, a dobrzy menedżerowie wiedzą, jak zmusić ludzi do reagowania.

tylerl
źródło
4

Myślę, że pozytywne opinie są bardzo ważne i wynikają głównie z osobistej dynamiki realpolitik. Wszyscy siedzimy i piszemy kod godzinami, dniami, tygodniami, miesiącami i większość z nas jest dumna z tego, co robimy. Recenzje kodu są szansą, aby to pokazać.

Jeśli przejdziesz do przeglądu kodu, a najlepszym rezultatem, na który możesz liczyć, jest „brak komentarza” (tzn. Brak równowagi pozytywnych opinii), spotkanie można łatwo nazwać perspektywicznym „Dowiedz się, jak źle ludzie myślą, że ssiesz”. W rezultacie programiści zaczną się denerwować lub nawet przerażać recenzjami kodu, co wyraźnie szkodzi zespołowi. Programiści „zapomną” poddać przeglądowi kod lub rozwiną wyuczoną bezradność i po prostu zapytają swoich stałych krytyków, co zrobić z każdym drobiazgiem, aby uniknąć wybuchu na tych spotkaniach.

Dobrze jest powiedzieć, że teoretycznie najbardziej logiczne jest naprawienie zła i poproszenie wszystkich, aby zostawili emocje przy drzwiach, ale właśnie takie postawy odpowiedzialne za to, że programiści stają się głuche interpersonalnie. Pomijając teorie, jesteśmy ludźmi i ludzie od czasu do czasu lubią klepać się po plecach, nawet nominalnie. To się liczy.

Erik Dietrich
źródło
Przypomina mi to koncepcję „Appreciative Enquiry”, która ma na celu ulepszenie i rozwinięcie tego, co już działa, zamiast koncentrować się na problemach do rozwiązania.
3

Jest to ważniejsze, jeśli robisz recenzje towarzyszące lub zespoły. W pisemnej recenzji brak wiadomości to dobra wiadomość. Celem jest wprowadzenie kodu do produkcji. Kiedy jest to twój kod, powinieneś czuć się dobrze ze sobą.

Przegląd kodu powinien być wykorzystywany jako źródło informacji pomagających w mentorowaniu i zarządzaniu zespołem. Istnieje wiele okazji do wyrażenia pozytywnej opinii bez zaśmiecania bazy danych recenzji kodu. Przykłady można wyciągnąć, aby udostępnić je innym.

Recenzowanie programisty to coś więcej niż tylko jego kod. Czas sprawdzania kodu przechwytującego może przynieść efekt przeciwny do zamierzonego, gdy chcemy wyciągnąć aplikację z domu. Ustaw czas, który jest specyficzny, aby pomóc programistom poza recenzjami kodu, ale nie oznacza to, że powinieneś wykluczyć opinie z recenzji kodu.

JeffO
źródło
2

Jedyny sposób, w jaki mogę wymyślić, gdzie pozytywne opinie na temat kodu mogłyby cię obrócić, to jeśli nie będziesz ostrożny, aby uniknąć „komplementu odręcznego”. Większość ludzi jest z tym zaznajomiona ... oznacza to zwrotami typu „Świetna robota, ale ...”

Jeśli wszyscy przyjdą na spotkanie z przekonaniem, że nie jest to osobisty przegląd programisty, ale próba ulepszenia praktyki kodowania dla jakości całego systemu, wówczas każda informacja zwrotna jest „dobra”. Informacje zwrotne, które podkreślają sposoby poprawy praktyki kodowania, stają się równie ważne, jak informacje zwrotne podkreślające nową przydatną metodę radzenia sobie z problemem.

Przynajmniej, jeśli ktoś nie podejmie takiej długości, należy podkreślić, że dążenie do wykonania cyklu „dobra opinia, zła opinia, dobra opinia, zła opinia” w ramach procesu przeglądu nastąpi tylko z to samo uczucie komplementu z ręki. Nie próbuj wymuszać dobrych informacji zwrotnych, staraj się wzmocnić dobry wysiłek i uzupełniać dziury w wiedzy.

Zwroty, z których nauczyłem się najwięcej przez lata:

  • „To ciekawe podejście. Co się stanie, jeśli będziemy musieli uwzględnić [inny przypadek użycia]?”
  • „Niezła próba! Czy wiesz, że mamy już taką metodę? Może powinniśmy przeprowadzić testy porównawcze, aby sprawdzić, które podejście jest bardziej wydajne”.
Jason M. Batchelor
źródło
2

Przepływ pracy, który najbardziej podobał mi się w recenzjach kodu, był następujący:

  1. Dev przesyła łatkę na listę mailingową / narzędzie online.
  2. Każdy, kto musi się tym przejmować, patrzy na łatkę, sugeruje ulepszenia.
  3. Dev wraca do # 1
  4. Jeśli nie jest wymagana poprawa, ludzie mówią: „Dobra robota, proszę się zaangażować”. <- POZYTYWNE OPINIE. Cały akceptowany kod jest dobry. Im mniej ludzie muszą ci mówić, aby zmieniać rzeczy, tym lepiej sobie radzisz.
  5. Dev zobowiązuje się, przechodzi do następnego elementu.

Zwykle zdarza się, że nowi deweloperzy otrzymują znacznie więcej „korekcyjnych” informacji zwrotnych, gdy zapoznają się z bazą kodu.

Korzyści z tego podejścia to:

  1. Wszyscy wiedzą, co wszyscy robią. Nie ma wiedzy monopolizującej ani tajemniczych.
  2. Każdy uczy się na podstawie opinii innych. To jest ważne. Jeśli informacje zwrotne zdarzają się tylko między 2 osobami podczas rozmowy twarzą w twarz podczas parowania, to osoba po drugiej stronie pokoju nie korzysta w taki sam sposób, jak gdyby to się stało na liście mailingowej.
  3. Inni deweloperzy mogą zwykle wykryć błędy, zanim przejdą do kontroli wersji.
MrFox
źródło
Zasadniczo masz nadzieję, że w ogóle nie otrzymasz żadnej opinii. Po co zawracać sobie głowę pracą? Możesz być niewidoczny w domu.
1

W ogóle się z tym nie zgadzam. Jaka jest różnica między Dobrymi Technikami Rozwoju a tak zwanymi Ninja Coders, którzy potrafią pisać niesamowity, ale niewytłumaczalny kod dla śmiertelników? Rozwój oprogramowania jest obecnie (IMO) dyscypliną o najniższym wspólnym mianowniku, w której unika się sprytu i sprytu na rzecz łatwości konserwacji i łatwości zrozumienia. To po prostu zbyt ryzykowne.

Nie mogę sobie wyobrazić czasu, w którym widziałem kod w recenzji, który sprawiłby, żebym powiedział „Och, to super”. Mogę tylko założyć, że gdybym napotkał taki kod, wpadłby do obozu Cool-Yet-Uncceptable.

Miałbyś również problemy z ludźmi, którzy nie otrzymali pozytywnej opinii, być może próbując zbyt mocno i ostatecznie robiąc bałagan „Zaufaj mi, to działa!”.

Przeglądy kodu służą rozłożeniu odpowiedzialności za jakość kodu na zespół, tzn. Indywidualnego programisty nie można obwiniać, jeśli później pojawi się poważny problem. Użyj go, aby znaleźć problemy, użyj go, aby uzyskać wyjaśnienia od oryginalnego twórcy dziwnych rzeczy na wypadek, gdybyś kiedyś musiał to utrzymywać. Osobiście jestem bardziej zainteresowany otrzymywaniem negatywnych opinii. Klienci nie dbają o fajność twojego kodu, tylko że robi to, czego chcą.

Pozostaw wsteczne uderzenie do pubu.

James
źródło
1
„Klienci nie dbają o fajność twojego kodu, tylko że robi to, co chcą”. Klienci również nie dbają o czytelność kodu. Oni mogą dbać o tym, jak długo to trwa, aby naprawić błąd, dodać funkcję, lub zmienić jakiś problem, ale na pewno nie dbają o czytelności kodu per se.
CVn
1

To miało dla mnie znaczenie. Nie chcę puchatych komentarzy ani pozytywności ze względu na pozytywność. Jeśli cały kod, który napisałem, jest kiepski, powiedz mi dlaczego, poprawmy go i nauczmy się. Ale jeśli zrobię coś dobrze, miło jest to usłyszeć raz na jakiś czas. Nie potrzebuję dodatniego wzmocnienia dla wszystkiego, co zrobiłem, to było „poprawne”, ale nawet jeśli jest to „poprawmy X, Y i Z, ale reszta wygląda dobrze”, to ma znaczenie.

peacedog
źródło
0

Nie tak ważne jak uczciwe opinie. Pracuję dla dużej korporacji finansowej, a nasi klienci nie dbają o to, czy programista stara się, czy jest dobrym facetem, czy zwykle pisze dobry kod! Wymagają oprogramowania, które działa.

aserwin
źródło
Z mojego doświadczenia wynika, że ​​programiści, którzy bardzo się starają, są „dobrymi facetami” i są zadowoleni ze wspierającego zespołu, który zazwyczaj pisze oprogramowanie, które działa.
c_maker,
Chyba kurczak i jajko. Pytanie dotyczyło jednak przeglądu kodu ... ale nie sądzę, żeby nadszedł czas na głaskanie ego ...
aserwin,
Przegląd kodu nie jest czasem na ustalenie, czy widoczne dla użytkownika części oprogramowania działają zgodnie ze specyfikacją.
CVn
0

Myślę, że ważne jest, aby być całkowicie obiektywnym. Próba zwiększenia morale poprzez wyrażanie pozytywnych komentarzy jest dla mnie stratą czasu.

Może to oznaczać, że recenzje kodu są nadmiernie krytyczne - ale nie o to chodzi. Powinniśmy także być wobec siebie krytyczni. Uważam, że założenie, że kod, który napisałem, jest prawdopodobnie kompletnym śmieciem i zdecydowanie można go ulepszyć, popycha mnie do poprawy mojego kodu i poziomu umiejętności.

Jeśli nie otrzymasz żadnych komentarzy, możesz uznać, że wykonałeś dobrą robotę.

użytkownik619818
źródło
Być może dlatego większość programistów to mężczyźni.
-1

Mantra jest prosta: jeśli ktoś chce Kodeksu jakości (który w rzeczywistości jest mniej), należy zastosować odpowiednie metody przeglądu. Mimo to pozytywne opinie pomagają programistom / programistom myśleć i wymyślać pomysły / rozwiązania / poprawki. Nie bądź zbyt surowy, ale bądź stanowczy w kwestii. Pytania i odpowiedzi Menedżerowie powinni być świadomi dobrych metod i praktyk, aby mógł on / ona poprowadzić zespół (lub członka) we właściwym kierunku. Daje to jakość. Kropka.

akula
źródło
-3

Jeśli kod jest przeznaczony na konkurs lub jest przesłany na rozmowę kwalifikacyjną (innymi słowy, kod jest napisany i nie może być przepisany), pozytywne komentarze są koniecznością. W rzeczywistości powinieneś upewnić się, że są pozytywne opinie (tam, gdzie to możliwe!), A także negatywne. W ten sposób programista wie, gdzie leżą jego mocne i słabe strony, i może to zrekompensować.

Wydaje się jednak, że rozmawiasz w środowisku pracy, w którym kod można przepisać. W takim przypadku próbujesz usunąć błędy z twojego systemu. W tej sytuacji tylko negatywne błędy mają jakąkolwiek wartość.

Jeśli czujesz się nieswojo, zwołaj cotygodniowe spotkanie poświęcone przeglądowi kodu, na którym każdy może omówić zarówno dobry, jak i zły kod.

EDYCJA: Chociaż powiem, że jeśli coś robi na tobie wystarczające wrażenie, nic nie powstrzyma cię przed osobistym wyrażeniem uznania. Wygląda jednak na to, że moduł śledzący służy wyłącznie do przeglądu kodu produkcyjnego.

KBKarma
źródło
6
„Więc w tej sytuacji tylko negatywne błędy mają jakąkolwiek wartość”. - W ogóle się z tym nie zgadzam. Jeśli ktoś wymyśli świetny sposób na naprawienie błędu / wdrożenie funkcji, nie jest to absolutnie bezwartościowe. Ważne jest utrzymanie motywacji. Jeśli wyróżnisz tylko awarię, będziesz mieć problemy.
Mat.
Zły wybór słów z mojej strony. Podczas gdy dobre rzeczy nie są bezwartościowe (mając wystarczająco dużo żargonu w moim czasie), błędy są najprawdopodobniej tym, dla czego został ustawiony tracker. OP może, jeśli tak zdecyduje, pozostawić pozytywne komentarze. Ale osobiście zostawiłbym to na rozmowy twarzą w twarz, ponieważ zapobiega to zatkaniu modułu śledzącego. Ponadto jestem bardzo zirytowany, że nie mogę głosować za komentarzami. :)
KBKarma,
1
@KBKarma - jeśli uważasz, że Twoja oryginalna odpowiedź nie została sformułowana tak dobrze, jak mogła, proszę wróć i edytuj odpowiedź, aby odpowiednio odzwierciedlić twoje myśli. Poszukaj przycisku edycji pod swoją odpowiedzią.