Code review? A na co to komu? :D
Choć szczerze żałuję, że go nie ma :(
- 1
- 2
- Rejestracja:ponad 5 lat
- Ostatnio:ponad 3 lata
- Postów:97
2
edytowany 1x, ostatnio: TheRottenCookie
- Rejestracja:około 17 lat
- Ostatnio:około 16 godzin
- Postów:1873
7
To jest w dużej mierze o dojrzałości. Należy sobie odpowiedzieć po co robimy code review. Na pewno nie po to, żeby manifestować „kto ma większego jsona” albo sprawdzać „kropki i przecinki”.
Fajnie, jak na CR pojawiają się również - uwaga - pozytywne komentarze i pochwały. Poza tym dobre CR, to sprawne CR - dopiero kod na produkcji przynosi $$$.
edytowany 1x, ostatnio: Charles_Ray
- Rejestracja:prawie 7 lat
- Ostatnio:3 dni
- Lokalizacja:Kraków
- Postów:1999
5
Charles_Ray napisał(a):
Poza tym dobre CR, to sprawne CR - dopiero kod na produkcji przynosi $$$.
O, podniosłeś dość ważny punkt.
Mało rzeczy w CR jest tak męczących, jak proszenie o review
dzień....
drugi....
tydzień....
Kiepska sprawa - potem te PRy się piętrzą i ktoś będzie niezadowolony, bo z gotowego PRa zrobi się taki, który trzeba rebase'ować na drugi koniec świata.
Teraz mam duży problem z Gitlabem, bo trzeba się nagrzebać, żeby odnaleźć wszystkie aktualne MRy otwarte w projektach Twojego zespołu. Czemu - nie wiem, może jest na to patent, którego nie znam lub dodatkowa licencja, której nie mam. W Bitbucket po prostu to miałem OOTB i nie musiałem kombinować. Efekt jest taki, że faktycznie przegapiam - ale w normalnej sytuacji staram się po prostu zacząć dzień od sprawdzenia, co jest aktualnie otwarte i zrobić komuś review. Zawsze to jakiś krok w kierunku odgruzowania ;)
edytowany 1x, ostatnio: superdurszlak
- Rejestracja:około 17 lat
- Ostatnio:około 16 godzin
- Postów:1873
3
Owner zadania powinien zabiegać o szybkie review, a jeśli się to nie dzieje - podnosić na retro. To musi chodzić jak w szwajcarskim zegarku, zespół musi być skoncentrowany na celu.
Jak na moje, przerzucanie się nawzajem spamem, żeby doprosić się w końcu o review nie sprzyja koncentracji :P
Jakim spamem? Nie rozumiem. Wystarczy ustalić kontrakt w zespole, jeśli coś się omsknie, to dopiero pisać. Musi być odpowiedzialność za domknięcie zadania, nikt inny o to nie zadba tak dobrze jak owner. To Tobie ma zależeć, nie koledze.
- Rejestracja:ponad 7 lat
- Ostatnio:około 11 godzin
- Postów:3277
10
Różnie bywało - w jednym projekcie, dzięki CR nabierałem bardzo szybko sprawności w technologii, w drugim miałem do czynienia z neofitami marudzenia wujka Boba i niekończące się dyskusje o tym jak dokładnie ma się nazywać jakaś metoda, wymyślane na zasadzie "widzimisię" zasady, czy lepiej pisać i > 0, czy i >= 1 i inne takie głupoty.
Moim zdaniem CR, żeby działało musi być:
- robione natychmiast, jak są jakieś CR do zrobienia, to nikt nie ma prawa brać nowego taska
- obejmować mały kawałek kodu. To jest generalnie problem, że najpierw są za duże taski, później za duże PR i zrobienie CR jest trudne i długie, a ewentualne zmiany po review potrafią wywrócić wszystko do góry nogami
- robiony porządnie, przynajmniej jeden recenzent musi być fachurą. Nie warto dawać za dużo ludzi do review, bo kończy się to tym, że wszyscy zrobią recenzję na odwal się.
zespół musi być świadomy po co się robi CR, czyli:
- ostatnia instancja wychwycenia błędów logicznych
- sprawdzenie jakości testów
- rozpowszechnienie w zespole wiedzy o różnych obszarach
- sprawdzenie, czy nowy kod pasuje do ustaleń w projekcie, np. czy endpointy pasują do RESTa, czy ktoś nie wpadł na pomysł użycia Gson, jak cały projekt siedzi w Jackson
- wychwycenie bałaganu, który jest trudny do wyłapania przez sonara - problemy z dziedziczeniem, naruszenia granic odpowiedzialności, błędny podział na pakiety
edytowany 1x, ostatnio: piotrpo
- Rejestracja:około 21 lat
- Ostatnio:prawie 3 lata
- Lokalizacja:Space: the final frontier
- Postów:26433
7
Ja osobiście nie potrzebuje pochwał w CR i uważam że są raczej zbędne, niemniej nie lubie jak dostaje approve bez słowa, bo wtedy trochę nie ufam, że ktoś serio poważnie na ten kod popatrzył. I w ogóle dziwią mnie trochę komentarze o porównywaniu kto ma większego jsona, bo ja osobiście po to wystawiam review żeby ktoś na ten kod popatrzył i może sie czegoś nauczę. Ba, w moim aktualnym zespole jestem osobą która niejako wprowadziła w ogóle praktykę robienia review :)
Zgadzam się z tym że branche muszą być małe - jak merge ma 50 linijek to będzie 10 sugestii na review, a jak ma 5000 to będzie tylko ok, spoko, approve bo ciężko ogarnąć cokolwiek.
Ludzie są na różnych etapach rozwoju zawodowego i emocjonalnego. Niektóre osobniki programistyczne są mocno przewrażliwione na swoim punkcie. Na ogół jest jak z moczeniem nocnym - z tego się wyrasta. W CR jest jeszcze standardowy problem komunikacji pisemnej, nie masz mowy ciała, tonu głosu i każda pierdoła potrafi urosnąć do rangi interpersonalnego problemu.
I to są właśnie te ważne umiejętności miękkie, na którym tak bardzo zależy HR'om
sprzedam luja ogłuszacza Na łazarskim rejonie nie jest kolorowo ( ͡° ͜ʖ ͡°)
@ledi12: nie inaczej, a jak nadal będzie się koleś stawiać na tych review to trzeba będzie zastosować kołowrotek...
- Rejestracja:ponad 5 lat
- Ostatnio:14 minut
- Postów:372
2
Pracuję w 3. firmie i wyglądało to tak:
Firma1: tu masz taski, tu repo, jak zrobisz taska to wrzucasz na developa i robisz kolejnego, 0 CR
Firma2: tu niby lepiej, było CR wykonywane przez typowego seniora przez zasiedzenie któremu nie chciało się za wiele robić, czasem coś skomentował, ale w 90% jak kod działał to było approve bez żadnego słowa.
Firma3: wrzucam pierwszy prosty task, a tu od razu 4 komentarze, a że tu metoda na 10 linii i można rozbić na dwie czytelniejsze, a tu że walimy kilka zapytań do bazy zamiast zrobić jedno i je przeiterować, itp., jestem mega zadowolony bo po tym CR kod jest superczytelny (podobnie jak cały projekt) i nie ma problemu w poruszaniu się w kodzie.
- Rejestracja:ponad 6 lat
- Ostatnio:około 2 godziny
- Postów:713
5
CR to podstawa. To jest ten moment kiedy możesz cię czegoś nauczyć i jeszcze ci za to zapłacą 15k.
W obecnej firmie jestem tylko dlatego że mam ziomka który jest robotem i zawsze znajdzie mi coś do poprawki -> ja dostaję hajs za to że on mi znajduje rzeczy które można poprawić. Płacą mi za naukę
Paździoch zawsze kombinowal
- 1
- 2
.andy