Wątek przeniesiony 2021-06-15 17:51 z Java przez Shalom.

Jak wygląda u Was w firmach code review?

5

Może gość po prostu nie potrafił zakomunikować powodu swojego zdenerwowania, a wykrzyknik był tylko jakimś tam punktem powiązanym z innymi Twoimi zachowaniami. Nie znam Cię, więc nie chcę oceniać pochopnie, po prostu rzuca mi się w oczy, że teraz piszesz o wykrzykniku w code review (o ile jest możliwość użycia go w kontekście neutralnym, to na ogół jednak wiąże się z negatywnymi emocjami wypływającymi ze strony piszącego i z niefajnymi sytuacjami). Prawdę mówiąc to ja sobie nie wyobrażam użycia wykrzyknika w czasie oceniania kogokolwiek, bo wykrzyknik służy do wyrażania rzeczy dość jednoznacznie nacechowanych, taka kultura, tak samo jak idąc na pogrzeb nie wypada iść w brudnym dresie. Do tego w tym temacie od razu nazwałeś kogoś "nerwuskiem" dlatego, że czuł się zdenerwowany Twoją oceną, a to też nie jest pozytywnie nacechowane słowo. Jesteś pewien, że to on jest nerwowy? Że to on jest źródłem negatywnych emocji w Waszych kontaktach? W innym temacie obruszałeś się nieznajomością podstaw, które Ty akurat znasz, w procesie rekrutacyjnym, nie mówiąc już o tym co sobie myśli osoba, która ma takie braki i chce zwyczajnej rynkowej pensji. Ja sobie nie wyobrażam jak można czymś takim się przejmować. Z mojej perspektywy to świadczy bardziej o Twojej nerwowości, a tutaj opisany przypadek wygląda raczej na negatywną emocjonalnie reakcję osoby, która poddawana jest kontaktom z osobą emocjonalną i wyrażającą negatywne emocje. Żeby też tylko sprawę wyjaśnić - ja tu nie chcę Ciebie oceniać. Ja w ogóle nic o Tobie nie wiem, a opinię wyrobiłem sobie na podstawie tylko trzech Twoich postów, co jest bardzo mocno obciążone błędem i wprost o tym mówię. Nie potrafię Cię ocenić, natomiast jeśli ogólnie w życiu prezentujesz takie podejście jak w tych postach, to może po prostu warto przeprowadzić refleksje nad samym sobą, bo może to nie jest tak, że to inni są winni wszystkiemu, a zwyczajnie nie czują się komfortowo w sytuacji, gdy ktoś do nich podchodzi negatywnie, więc przesadnie (z drugiej strony - może to nie jest przesadna reakcja, a właśnie właściwa reakcja obronna pozwalająca na to, by unikać kontaktu z tym, co wpływa na życie negatywnie) reagują, a ich głównym problemem z Twojej perspektywy jest przede wszystkim to, że nie potrafią Ci zakomunikować faktycznych przyczyn swojego zachowania? Ja to podkreślam jeszcze raz - to jest bardzo obciążona błędem analiza, nie chcę tu Cię oceniać (a już na pewno nie Twoje kwalifikacje techniczne), ale po prostu w życiu widziałem sporo osób, które w życiu podchodziły do innych osób w sposób, który w Twoich postach wydaję się widzieć, co końcowo powodowało bardzo toksyczną i nerwową atmosferę u wszystkich stron biorących udział w interakcji.

5
Kiko88 napisał(a):

po czym jakiś junior publicznie próbuje go zniżyć do parteru, bo javadoc nie dołączył. Powtórzę: proces nie działa.

No jeśli CR polega na wzajemnym zniżaniu do parteru i pokazywaniu sobie, kto ma większego, to problem jest w podejściu a nie w procesie. CR powinno mieć na celu doprowadzenie PR do lepszego stanu, a nie publiczne poniżanie kogoś.

Z drugiej strony, jeśli ktoś każdą krytykę, a w szczególności od osób młodszych stażem odbiera jako próbę upokorzenia, no to nie jest zdrowe podejście.

0

@superdurszlak: Przedstawiłeś bardzo toksyczną sytuację, która faktycznie jest obecna w społeczeństwie. Oczywiście, że seniorzy noszą w spodniach coś na wzór baobabów, ale nawet oni popełniają w kodzie ba(o)bole.

No cóż, grunt to zrozumieć co ma na celu krytyka. Natomiast inną sprawą jest to, że łatwiej ją zrozumieć, gdy jest uargumentowana.

0

@superdurszlak: Szczerze jego komentarze to w większości strata czasu, no nie jest bystrzakiem niestety, a proces jest naprawdę bardzo słaby.

6
  1. Co do konkretnego przypadku. To not nulle javadoce i inne to są rzeczy, które rozwiązuje się na poziomie buildu/CI. Ustawia się z zespołem reguły i wbija w lintera. Do PR takie rzeczy nie dochodzą. Albo inaczej - trafiają max 2-3 razy - jak się coś powtarza i jest na to reguła to leci do lintera. (btw. nie mamy żadnej reguły na javadoce - przeważnie żadnych i to jest raczej norma - o ile nie piszesz biblioteki). Jak nie masz CI i pracujesz z baranami to sam sobie CI zrób. Nawet pluginy do IDE pomogą (choć wsparcie przez IDE to prawie najgorsze rozwiązanie na "code style").
  2. Mimo to code review nie są efektywne - w praktycznie każdej firmie, w jakiej pracuje lub pracowałem - z jednego prostego powodu - za duże. Zadanie ukończone z punktu widzenia biznesu to często za dużo kodu, żeby osoba postronna miała szanse ogarnąć w rozsądnym czasie co się tam działo. Więc na code review łapane są drobnostki, ale nie duże zwały ideologiczne.
4

Ja od pewnego czasu code review robię zupełnie inaczej niż kiedyś. Po pierwsze, nie szukam błędów w kodzie. Ok, czasem bywa że znajdę babola przypadkiem, bo coś rzuci się w oczy. Sprawdzam za to czy są testy i co testują.

Jednak główny nacisk kłade na to czy kod jest łatwy w zrozumieniu i czy wprowadzone abstrakcje mają sens. Także jak widzę nową klasę na 1500 linii i zero komentarza do czego ona służy i gdzie się wpasowuje w całość systemu to często nawet nie czytam tego kodu tylko odwalam i już.

Na styl wciec też nie patrzę bo od tego jest formatter.

2

Wcześniej miałem doświadczenie z bardzo sensownymi CR. Ale teraz mam taki zespół, że bez zastanowienia piszą jak oni by napisali jakaś "linijke" i poza tym, że to co napisali jest bez sensu ale wyglada bardziej fancy to nic nie wnosi. Jeszcze mam drugi przypadek gdzie robię ze starszym niemcem to tam mam komentarze typu zebym nie używał zbyt nowoczesnych konstrukcji jak optional, czy functional interface, forEach bo on tego nie rozumie. Albo jak sobie coś opakuje ładnie w abstrakcje to już muszę walczyć czemu tu zrobiłem jakiegoś małego builderka zamiast 15 ifów w klasie która ma 4k lini xD.
Mam też taki zwyczaj, że jak widzę zakomentowany kod to zazwyczaj go wypieprzam a jak juz widze ze został zakomentowany przed 2015 to nawet nie czytam co tam jest i ostatnio dostałem komentarz zebym tak nie wywalal wszystkie bo jak bedzie potrzebne to nie znajdzie xD (tak używamy gita).

0

@Schadoow: Pierwsza część Twojej wypowiedzi mogłaby również opisywać sytuację u nas. Doładnie chodzi często, że tę linijkę można napisać inaczej. Z reguły dokomentowuję: dróg do celu jest wiele.

0

@MarekR22: Nie ma emocji z mojej strony. Żadnych. Koloryzujesz.

3
Kiko88 napisał(a):

@superdurszlak: Szczerze jego komentarze to w większości strata czasu, no nie jest bystrzakiem niestety, a proces jest naprawdę bardzo słaby.

Jak na CR też mu mówisz, że nie jest bystrzakiem i tracisz na niego czas, to nie zdziw się jak będziesz miał potem konflikty w zespole :)

Skoro czyjś feedback na CR jest taki słaby to można pewnie znaleźć powód inny niż "jest debilem":

  • uważa złą / zbędną praktykę za dobrą, odwrotnie pewnie też
  • skoro tak, to pewnie nie zna tych dobrych praktyk lub nie wie, kiedy jakie stosować
  • może coś mu świta, ale nie umie dobrze uzasadnić swojego zdania
  • jeśli nie zna albo nie umie uzasadniać, to trzeba odesłać do źródeł - do skutku
  • jak zna i nie umie stosować, to pewnie dlatego, że jest juniorem i trudno mieć mu to za złe
  • dopóki wszystkie jego komentarze spławiasz "bo tak" / "bo mnie zniża do parteru", zamiast merytorycznie, to nic się nie zmieni :)

Jak się obrazi o czysto merytoryczne argumenty (bez łatek.w stylu nie jest bystrzakiem!) - to już nie Twój problem, Ty masz czyste konto i podkładkę, że próbowałeś.

1 użytkowników online, w tym zalogowanych: 0, gości: 1, botów: 0