Jak poradzić sobie z nieporozumieniem podczas przeglądu kodu dotyczącego mało prawdopodobnego przypadku na krawędzi?


188

Pracuję w startupie robotyki w zespole zajmującym się obsługą ścieżek i po przesłaniu żądania ściągnięcia mój kod jest sprawdzany.

Mój kolega z zespołu, który pracuje w zespole od ponad roku, skomentował mój kod, sugerując, że wykonuję o wiele więcej pracy, niż uważam za konieczne. Nie, nie jestem leniwym programistą. Uwielbiam elegancki kod, który ma dobre komentarze, nazwy zmiennych, wcięcia i poprawnie obsługuje przypadki. Ma jednak na myśli inny rodzaj organizacji, z którą się nie zgadzam.

Podam przykład:

Spędziłem dzień, pisząc przypadki testowe dotyczące zmiany algorytmu znajdowania przejścia, którego dokonałem. Zasugerował, że zajmę się niejasną sprawą, która jest bardzo mało prawdopodobna - w rzeczywistości nie jestem pewien, czy to możliwe. Utworzony przeze mnie kod działa już we wszystkich naszych oryginalnych przypadkach testowych i niektórych nowych, które znalazłem. Utworzony przeze mnie kod przechodzi już ponad 300 symulacji, które są uruchamiane co noc. Jednak poradzenie sobie z tą niejasną sprawą zajęłoby mi 13 godzin, które lepiej byłoby poświęcić na poprawę wydajności robota. Żeby było jasne, poprzedni algorytm, którego używaliśmy do tej pory, również nie zajmował się tym niejasnym przypadkiem i ani razu, w 40 000 wygenerowanych raportach, nigdy nie wystąpił. Jesteśmy startupem i musimy opracować produkt.

Nigdy wcześniej nie miałem recenzji kodu i nie jestem pewien, czy jestem zbyt kłótliwy; czy powinienem być cicho i robić to, co on mówi? Postanowiłem nie spuszczać głowy i po prostu dokonać zmiany, mimo że zdecydowanie nie zgadzam się z tym, że to był dobry użytek z czasu.


Szanuję mojego współpracownika i uznaję go za inteligentnego programistę. Po prostu się z nim nie zgadzam i nie wiem, jak poradzić sobie z nieporozumieniem podczas przeglądu kodu.

Wydaje mi się, że odpowiedź, którą wybrałem, spełnia te kryteria wyjaśniania, w jaki sposób młodszy programista może poradzić sobie z nieporozumieniem podczas przeglądu kodu.


152
Zdajesz sobie sprawę, że testy jednostkowe mają częściowo na celu wykrycie tych błędów na milion, gdy ktoś sprawdza zmianę w kodzie, która łamie go w jakiś niejasny sposób, prawda? Testy jednostkowe polegają nie tylko na upewnieniu się, że Twój kod jest teraz poprawny , ale także za trzy lata, gdy ktoś inny utrzymuje kod, który napisałeś.

189
@Klik „Rzeczywisty wkład nigdy nie będzie taki” To jest miejsce, w którym się mylisz. Spotkałem zbyt wiele przypadków „wkładu nigdy nie będzie tak” i byłem zaskoczony, gdy było „tak”. W środowisku produkcyjnym mogą się zdarzyć różne dziwne rzeczy. Nie myśl tylko o tym, jak będzie działać twoje oprogramowanie, ale także o tym, jak zawiedzie?

38
@Snowman Co więcej, przeglądy kodów punktowych mają częściowo na celu uchwycenie tych milionowych defektów, których nie znajdują testy jednostkowe, a nawet testy losowe / fuzzing.
Derek Elkins

11
Warto również pamiętać, że recenzje kodu służą nie tylko do wychwytywania problemów, ale także do nauki, w jaki sposób możesz lepiej, więc traktuj je jako okazję do nauki.
Steve Barnes

72
hej @Klik, wygląda na to, że podstawowym problemem jest to, że „boisz się mówić swoim umysłem”. NIGDY nie gniewaj się, ZAWSZE mów w myślach - z uśmiechem. Powinieneś był natychmiast powiedzieć facetowi: „Hmm, zajmie mi to co najmniej dwa dni, czy to jest tego warte, zapytajmy naszego szefa?” a po drugie należy powiedzieć „Nie zapomnij człowieka pracujemy nad robotyką, to faktycznie nie jest możliwe dla czujnik lewo, aby być na prawo od prawego czujnika - Zapytajmy szefa ile anty-fizyczne przypadki rogu chcemy na pokrycie ". Twój problem jest twój , twoim problemem jest to, że musisz zamienić gniew na rozmowę .
Fattie

Odpowiedzi:


227

niejasny przypadek, który jest niezwykle mało prawdopodobny - w rzeczywistości nie jestem pewien, czy to możliwe

Nieprzestrzeganie zachowań w kodzie może być bardzo ważne. Jeśli fragment kodu zostanie uruchomiony np. 50 razy na sekundę, szansa na milion pojawi się mniej więcej co 5,5 godziny działania. (W twoim przypadku szanse wydają się niższe).

Możesz porozmawiać o priorytetach ze swoim przełożonym (lub kimkolwiek, kto jest osobą starszą odpowiedzialną za jednostkę, w której pracujesz). Lepiej zrozumiesz, czy np. Praca nad wydajnością kodu lub kodowanie kuloodporne jest najwyższym priorytetem i jak nieprawdopodobne może być to narożnik. Twój recenzent może mieć również wypaczone pojęcie priorytetów. Po rozmowie z osobą odpowiedzialną łatwiej będzie ci (nie) zgodzić się z sugestiami recenzenta i będziesz mieć do czego się odwołać.

Zawsze warto mieć więcej niż jednego recenzenta. Jeśli Twój kod jest sprawdzany tylko przez jednego kolegę, poproś kogoś innego, kto zna ten kod lub bazę kodów ogólnie, o sprawdzenie. Druga opinia ponownie pomoże ci łatwiej (nie) zgodzić się z sugestiami recenzenta.

Posiadanie wielu powtarzających się komentarzy podczas kilku przeglądów kodu zwykle wskazuje na to, że większa rzecz nie jest wyraźnie komunikowana, a te same problemy pojawiają się wielokrotnie. Spróbuj znaleźć tę większą rzecz i przedyskutuj ją bezpośrednio z recenzentem. Zapytaj wystarczająco dlaczego pytania. Bardzo mi pomogło, kiedy zacząłem ćwiczyć recenzje kodu.


9
@ 9000 Może to być frustrujące, gdy odpowiedź brzmi „ponieważ życie jest takie”. Na przykład ostatnio musiałem przejść przez: „Używaj spacji nad kartami”. "Dlaczego?" „Ponieważ nasz przewodnik po stylu mówi tak.” "Dlaczego?" „Nie wiem; nie napisałem tego”. „Ha, najwyraźniej się mylisz, a ja mam rację, i zamierzam zostawić swój kod z kartami!” Potem haczyk po zatwierdzeniu i tak to zmienił, ale mimo to - jeśli użyjesz techniki 5 dlaczego, idź, aż uzyskasz rozsądną odpowiedź, nie dopóki nie sprawisz, że druga osoba będzie sfrustrowana.
Nic Hartley,

111
@QPaysTaxes: Każdy powinien wiedzieć, dlaczego przestrzeni ponad zakładkami (lub zakładek ponad spacjami) argumentem: ponieważ obie są poprawne, ale konsystencja jest bardziej ważne, więc po prostu wybrać jedną, być spójne i nie trać czasu bikeshedding
slebetman

30
Not having untested behaviors in code can be very important. If a piece of code is run e.g. 50 times a second, a one in a million chance will happen approximately every 5.5 hours of runtime. (In your case, the odds seem lower.)-- Co? Nie. Chyba że prowadzisz symulację Monte-Carlo lub twój kod zawiera jakiś losowy element. Komputery nie uruchamiają oprogramowania zgodnie z krzywą dzwonka lub odchyleniem standardowym, chyba że wyraźnie im to nakazujesz.
Robert Harvey,

10
@RobertHarvey: rozważ coś w rodzaju wyścigu, który jest dokładnie losowym elementem. Weź również pod uwagę błąd zależny od danych podczas przetwarzania danych przesyłanych strumieniowo; podczas gdy dane mogą nie być dość losowe, ale jeśli nie są bardzo powtarzalne, określona kombinacja bajtów może występować wielokrotnie. Jeden na milion = wywołany przez konkretną kombinację 20 bitów, taki błąd byłby natychmiast widoczny, jeśli przetwarzałby strumień wideo; coś, co zdarza się rzadko, ale regularnie może obejmować np. 40 bitów.
9000

7
@RobertHarvey: Całkiem możliwe! Chciałem tylko wskazać, że istnieją fragmenty kodu, które mogą mieć szansę 1e-6 (nie 0 i nie 1) na złamanie pod konkretnym wywołaniem, odnosząc się do „niejasnego przypadku, który jest bardzo mało prawdopodobny”. Chociaż takie błędy zwykle nie są widoczne podczas testów, widoczne podczas produkcji.
9000

323

Mogę podać przykład sprawy narożnej, która nigdy nie mogłaby wystąpić, która spowodowała katastrofę.

Podczas opracowywania Ariane 4 wartości z bocznych przyspieszeniomierzy były skalowane, aby pasowały do 16-bitowej liczby całkowitej ze znakiem i ponieważ maksymalna możliwa moc akcelerometrów, po skalowaniu, nigdy nie może przekroczyć 32767, a minimum nigdy nie może spaść poniżej - 32768 „nie było potrzeby sprawdzania zasięgu”. Zasadniczo wszystkie dane wejściowe powinny być sprawdzane przed każdą konwersją, ale w tym przypadku byłoby to próbą uchwycenia niemożliwego przypadku narożnego.

Kilka lat później opracowywano Ariane 5 , a kod skalowania bocznych akcelerometrów został ponownie wykorzystany przy minimalnym testowaniu, ponieważ został „sprawdzony w użyciu”. Niestety, większa rakieta mogła oczekiwać większych przyspieszeń bocznych, więc akcelerometry zostały zaktualizowane i mogły wytwarzać większe 64-bitowe wartości zmiennoprzecinkowe .

Te większe wartości „zawinięte” w kod konwersji, nie pamiętają sprawdzania zakresu, a wyniki pierwszego uruchomienia w 1996 roku nie były dobre. Kosztowało to miliony firmy i spowodowało dużą przerwę w programie.

Wpisz opis zdjęcia tutaj

Chodzi mi o to, aby nie ignorować przypadków testowych, ponieważ nigdy się nie zdarzają, są bardzo mało prawdopodobne itp .: standardy, dla których kodowały, wymagały sprawdzenia zakresu wszystkich zewnętrznych wartości wejściowych. Gdyby to zostało przetestowane i załatwione, katastrofie można by zapobiec.

Zauważ, że w Ariane 4 nie był to błąd (ponieważ wszystko działało dobrze dla każdej możliwej wartości) - było to niezgodne ze standardami. Gdy kod został ponownie użyty w innym scenariuszu, zawiodł katastrofalnie, podczas gdy gdyby zakres wartości został przycięty, prawdopodobnie zawiodłby z wdziękiem, a istnienie przypadku testowego dla tego mogłoby spowodować przegląd wartości. Warto również zauważyć, że podczas gdy koderzy i testerzy zgłosili się do krytyki ze strony śledczych po wybuchu, odpowiedzialność za zarządzanie, kontrolę jakości i przywództwo spoczywała w większości.

Wyjaśnienie

Chociaż nie całe oprogramowanie ma kluczowe znaczenie dla bezpieczeństwa, ani nie jest tak spektakularne, gdy zawodzi, moim zamiarem było podkreślenie, że „niemożliwe” testy mogą nadal mieć wartość. To najbardziej dramatyczny przypadek, jaki znam, ale robotyka może również przynieść katastrofalne skutki.

Osobiście powiedziałbym, że gdy ktoś zwróci uwagę zespołowi testowemu na przypadek narożny, należy przeprowadzić test, aby to sprawdzić. Kierownik zespołu wdrażającego lub kierownik projektu może zdecydować, że nie będzie próbować zaradzić stwierdzonym awariom, ale powinien zdawać sobie sprawę z istnienia niedociągnięć. Alternatywnie, jeśli testowanie jest zbyt skomplikowane lub kosztowne, aby go wdrożyć, można poruszyć problem w każdym używanym module śledzącym i / lub rejestrze ryzyka, aby wyjaśnić, że jest to niesprawdzony przypadek - że może być konieczne rozwiązanie przed zmianą użytkowania lub zapobiec niewłaściwemu użyciu.


102
O mój Boże, ta odpowiedź jest właśnie taka. OP zajmuje się oprogramowaniem, które ma reperkusje fizyczne. Bar został podniesiony. Recenzent prawdopodobnie nie zdawał sobie sprawy z tego „przypadku na krawędzi”, dopóki ponownie nie spojrzał na kod. A biorąc pod uwagę wystarczająco dużo czasu, nic nie jest przewagą. Nie chcesz przypadkowo wbić ramienia robota w czyjąś głowę (nie wiem, że ramię robota jest związane z tym kodem).
Greg Burghardt

11
Greg i Steve, to świetny przykład, ale to przykład błędu . To prosty, niejasny błąd. Dosłownie „błąd”, dzielony przez zero. Kiedy pracujesz nad algorytmami robotyki, jest to centralna idea, która myśli o koncepcjach możliwych fizycznie i niemożliwych. (Jeśli chcesz, „jeszcze nie” fizycznie możliwe koncepcje, z obecnymi urządzeniami). Zamieszanie między dwoma twórcami, o których tu mowa, wynika z tego, że (bardzo zaskakująco) nie są zgodni z tym paradygmatem. Cały ich zespół ma poważne problemy, jeśli im bardziej starsi koledzy nie zastosują tego paradygmatu.
Fattie

11
Myślę, że istnieją przykłady ze świata rzeczywistego, które dowodzą, dlaczego należy uwzględnić przypadki skrajne, ale to nie jest jeden z nich. Przytoczony przykład dotyczy użycia niewłaściwej biblioteki do zadania. Kod banana nie powinien być ślepo używany na pomarańczy. To wina osoby, która użyła kodu bananowego na pomarańczy, a nie osoby, która napisała kod pomarańczy, który nie poradziłby sobie z bananem. (Musiałem zamienić Apple na banan w tej analogii ze względu na dużą firmę technologiczną na rynku ...)
Origin

51
@JoeBlow Błąd, tak, ale można go uniknąć, taki, który mógłby zostać złapany za pomocą dodatkowych przypadków testowych i / lub recenzji kodu. Bóg wie tylko, czy w pewnym momencie był tam facet, który powiedział: „Wiesz, myślę, że powinniśmy zweryfikować ten zakres ...”, a inni mówią: „Nie martw się tym… co może pójść nie tak z niemożliwy przypadek? ” Jeśli błędy nie są wystarczającym dowodem na to, że nasza logika ma więcej luk niż chcielibyśmy, to nie wiem, co powiedzieć ...
code_dredd

30
Chodzi o to, że nie powinieneś ignorować przypadków testowych, ponieważ nigdy się nie zdarzają, jest to bardzo mało prawdopodobne itp., Standardy, dla których kodowały, wymagały sprawdzenia zakresu wszystkich zewnętrznych wartości wejściowych. Gdyby to zostało przetestowane i załatwione, katastrofie można by zapobiec. Zauważ, że w Ariane 3 nie był to błąd, to było nieprzestrzeganie standardów. Gdyby kod został ponownie użyty w innym scenariuszu, gdyby zawiódł katastrofalnie, a jeśli zakres wartości zostałby przycięty, prawdopodobnie zawiodłby z wdziękiem.
Steve Barnes,

84

Ponieważ nie było to wcześniej obsługiwane, nie można tego zrobić. Ty lub twój kolega możecie zapytać swojego przełożonego, czy warto podjąć wysiłek, aby omówić tę sprawę.


19
Myślę, że jest to kluczowa kwestia - chociaż uwzględnienie dodatkowego przypadku jest rzeczywiście użytecznym i ważnym usprawnieniem, nie ma potrzeby, aby z różnych powodów zamieniał się on w istniejący PR.
DeadMG

6
Niezależnie od tego, czy było to obsługiwane wcześniej, jest również nieistotne dla IMO, po prostu nie było wcześniej w opisie zadania.
Jasper N. Brouwer

6
Aby powtórzyć ten sentyment, dobrą odpowiedzią na komentarze w komentarzu może być: „Podniosę bilet, aby to opisać, dobra uwaga”. To potwierdza, że ​​„rzecz” musi zostać wykonana, jednocześnie umożliwiając jej nadanie priorytetu, obok wszystkich innych prac, które należy wykonać.
Edd

17
Nie można bardziej zdecydowanie się nie zgodzić. Fakt, że ten problem mógł nie zostać podniesiony podczas początkowej implementacji funkcji, może równie łatwo - ponieważ nie znamy szczegółów - może wskazywać na to, że do tej pory miał niewiarygodne szczęście, ponieważ może być niepotrzebny. Przegląd kodu modyfikacji, która zmienia sposób działania tego algorytmu - potencjalnie zwiększając prawdopodobieństwo wystąpienia tego przypadku krawędzi - jest właśnie czasem, aby poruszyć potencjalne problemy. O tym, czy jest to poza zakresem, należy zdecydować po właściwej ocenie dokładnie tego, a nie bez decyzji z małym kontekstem.
Mateusz

6
To okropna rada! Since it wasn't handled before, it's out of the scope for your effortwystarczyłoby zaznaczyć każdy raport o błędzie jako wontfix, zakładając, że twoja specyfikacja była na tyle zła, że ​​na początku nie brała pod uwagę przypadków skrajnych, nawet jeśli masz odpowiednią specyfikację.
Filip Haglund,

53

Dzięki złożonym algorytmom bardzo trudno jest udowodnić, że pomyślałeś o każdym przypadku testowym, który pojawi się w prawdziwym świecie. Kiedy celowo zostawiasz uszkodzoną skrzynkę testową, ponieważ nie pojawi się ona w prawdziwym świecie, potencjalnie pozostawiasz uszkodzone inne skrzynki testowe, o których nawet jeszcze nie pomyślałeś.

Innym często występującym efektem jest obsługa dodatkowych przypadków testowych, algorytm z konieczności staje się bardziej ogólny, dzięki czemu można znaleźć sposoby uproszczenia go i uczynienia go bardziej niezawodnym dla wszystkich przypadków testowych. Oszczędza to czas na konserwację i rozwiązywanie problemów na drodze.

Ponadto istnieje wiele przypadków, w których na wolności występują błędy. Wynika to z faktu, że Twój algorytm może się nie zmienić, ale jego dane wejściowe mogą ulec zmianie w kolejnych latach, gdy nikt nie pamięta o tym jednym nieobsługiwanym przypadku użycia. Właśnie dlatego doświadczeni programiści zajmują się tego rodzaju rzeczami, gdy są świeżo zaprzątnięci. Wróci cię ugryźć później, jeśli tego nie zrobisz.


7
Bardzo dobrze robisz w swoim drugim akapicie. Doświadczyłem tego już wcześniej, ale jego artykułowanie jest bardzo pomocne.
Klik

7
Trzeci przypadek = bingo. Pracuję głównie w starszym kodzie, i są projekty, w których 80% pracy to po prostu ustalanie miejsc, które przyjęły założenia. Podoba mi się ta odpowiedź, ale dodam, że czasem dobrze jest pokryć tak niemożliwy przypadek krawędzi, ustawiając wdzięczną awarię z dokładnym i szczegółowym komunikatem o błędzie, szczególnie gdy jesteś w kryzysie czasowym.
Jeutnarg,

Lubię rejestrować wyjątki, które mówią „[Opis błędu] Zgodnie ze specyfikacją [wersja] podpisana przez [osobę, która to wylogowała] nie może się zdarzyć”.
Peter Wone

Ale ponieważ wcześniej nie było dla niego przypadku testowego, kod (zakładając, że specyfikacja miała zastąpić poprzednią) nie powinien być dopasowany do nowych przypadków testowych w tej iteracji. A jeśli nie ma już testu dla tego narożnego przypadku, powinien to być nowy bilet / zadanie do wykonania, a nie opinia na temat recenzji kodu imo.
kb.

38

To nie jest pytanie techniczne, ale decyzja dotycząca strategii biznesowej. Zauważasz, że sugerowana poprawka dotyczy bardzo niejasnego przypadku, który prawie nigdy się nie wydarzy. To robi dużą różnicę, jeśli programujesz zabawkę lub programujesz powiedzmy sprzęt medyczny lub uzbrojony dron. Konsekwencje rzadkiej awarii będą bardzo różne.

Dokonując recenzji kodu, powinieneś zastosować podstawową wiedzę na temat priorytetów biznesowych przy podejmowaniu decyzji, ile zainwestować w obsługę rzadkich przypadków. Jeśli nie zgadzasz się ze swoim kolegą w kwestii interpretacji priorytetów firmy, możesz chcieć porozmawiać z kimś po stronie biznesowej rzeczy.


5
Dokładnie, zapytaj kogoś innego, czy warto to pokryć. Przynajmniej napisałbym przypadek testowy, a następnie oznaczyłem go jako „pominięty” z komentarzami stwierdzającymi, że ktoś inny podjął decyzję, że nie warto go wdrażać. CYA
Sean McSomething

2
Tak, za każdym razem, gdy pojawia się coś dużego, poza zakresem, ale nie pilny, podczas przeglądu kodu, staramy się stworzyć problem w naszym trackerze, a następnie nadać mu priorytet, podobnie jak resztę innych rzeczy, które musimy zrobić. Czasami oznacza to, że zadanie zostaje wygnane na drugi koniec listy priorytetów, ale jeśli tak jest, to naprawdę nie jest ważne.
Tacroy

1
„To nie jest szansa, to stawka!”
user1068

2
To najlepsza odpowiedź. Pytanie naprawdę dotyczy ustalania priorytetów i zarządzania ryzykiem.
wrschneider

Zgadzam się z tą decyzją biznesową. Stwórz bilet z szacunkowym czasem potrzebnym na jego naprawę, przypisz go swojemu szefowi (ktokolwiek w łańcuchu dowodzenia jest odpowiedzialny za przydzielenie twojego czasu) i poproś o przypisanie mu priorytetu (lub odrzucenia, w zależności od przypadku może być)
Matija Nalis

21

Przeglądy kodu nie dotyczą wyłącznie poprawności kodu. W rzeczywistości jest to znacznie poniżej listy korzyści, za dzieleniem się wiedzą i powolnym, ale stałym procesem w kierunku wypracowania konsensusu w stylu zespołu / projektu.

Kiedy doświadczasz, to, co liczy się jako „prawidłowe”, jest często dyskusyjne i każdy ma swój własny punkt widzenia na to, co to jest. Z mojego doświadczenia wynika, że ​​ograniczony czas i uwaga mogą powodować, że recenzje kodu są bardzo niespójne - ten sam problem może zostać wykryty lub nie w zależności od różnych programistów i w różnych momentach przez tego samego programistę. Recenzenci myślący „co poprawiłoby ten kod?” często sugerują zmiany, których nie dodaliby naturalnie do własnych wysiłków.

Jako doświadczony programista (ponad 15 lat jako programista) często jestem oceniany przez programistów z mniejszym doświadczeniem ode mnie. Czasami pytają mnie o zmiany, z którymi delikatnie się nie zgadzam lub które uważam za nieistotne. Nadal jednak wprowadzam te zmiany. Walczę o zmiany tylko wtedy, gdy uważam, że efekt końcowy spowodowałby słabość produktu, w którym koszt czasu jest bardzo wysoki lub gdy obiektywny punkt widzenia może być obiektywny (np. Prośba o zmianę wygrana) t pracować w języku, którego używamy, lub test porównawczy pokazuje, że deklarowana poprawa wydajności nie jest jednym).

Sugeruję więc ostrożnie dobierać swoje bitwy. Dwa dni kodowania przypadku testowego, który Twoim zdaniem nie jest konieczny, prawdopodobnie nie jest wart czasu / wysiłku na walkę. Jeśli korzystasz z narzędzia do sprawdzania, takiego jak żądania ściągania GitHub, możesz tam skomentować postrzegane koszty / korzyści, aby odnotować swój sprzeciw, ale zgodzić się na kontynuowanie pracy. To liczy się jako delikatne odsunięcie, więc recenzent wie, że przekraczają granicę, a co ważniejsze, dołącz swoje uzasadnienie, aby przypadki takie mogły zostać eskalowane, jeśli wpadną w impas. Chcesz uniknąć eskalacji pisemnych nieporozumień - nie chcesz spierać się na forum internetowym o różnice w pracy - więc pomocne może być omówienie problemu w pierwszej kolejności i zapisanie rzetelnego podsumowania wyników dyskusji,przyjazna dyskusja i tak zadecyduje dla was obojga).

Po wydarzeniu jest to dobry temat do przeglądu sprintu lub spotkań zespołu programistów itp. Przedstaw go tak neutralnie, jak to możliwe, np. „Podczas przeglądu kodu programista A zidentyfikował ten dodatkowy przypadek testowy, napisanie go zajęło dwa dni, zespół uważa, że ​​dodatkowe pokrycie było uzasadnione tym kosztem? ” - to podejście działa znacznie lepiej, jeśli faktycznie wykonujesz pracę, ponieważ pokazuje to w pozytywnym świetle; wykonałeś pracę i po prostu chcesz sondować zespół pod kątem niechęci do ryzyka w porównaniu z rozwojem funkcji.


2
„Prezentuj to tak neutralnie, jak potrafisz”… całkiem. Chciałbym pójść dalej niż NeilS i zasugerować, jak mówią, trzeba zamienić „gniew na rozmowę”. Po prostu natychmiast i otwarcie powiedz (na przykład w konkretnym przykładzie): „Steve, twoja skrzynka narożna jest niefizyczna z obecnym projektem mechanicznym, nie sądzisz, stary? Najpierw zdecydujmy, czy to niefizyczne, i nawet jeśli to jest to, czy warto spędzić dwa dni ”.
Fattie

1
„Jeśli korzystasz z narzędzia przeglądu” to kluczowa obserwacja. IME, narzędzia te są odpowiednie do prowadzenia rejestru recenzji, ale prawdziwa praca jest wykonywana osobiście podczas dyskusji. To najbardziej wydajny sposób na dwukierunkowy przepływ informacji. Jeśli nie zgadzasz się z recenzją, miej konstruktywnie tę osobowość osobiście, a dopiero potem wprowadź uzgodniony wynik do narzędzia.
Toby Speight

@TobySpeight: Zgadzam się i starałem się włączyć to do odpowiedzi.
Neil Slater,

1
2 godziny to więcej, z czym nie walczyłbym. 2 dni i nie będę w stanie ukończyć wszystkich innych zadań, które zobowiązałem się na tydzień.
Mateusz

15

Radziłbym przynajmniej twierdzić przeciwko niejasnej sprawie. W ten sposób nie tylko przyszli programiści zobaczą, że aktywnie zdecydowałeś się na tę sprawę, ale przy dobrej obsłudze awarii, która powinna już być na miejscu, przyniosłoby to również niespodzianki.

A następnie wykonaj test, który potwierdza tę awarię. W ten sposób zachowanie jest lepiej udokumentowane i pojawi się w testach jednostkowych.

Ta odpowiedź oczywiście zakłada, że ​​twoja ocena nawet bycia „wyjątkowo mało prawdopodobnym, jeśli to w ogóle możliwe” jest poprawna i nie możemy tego ocenić. Ale jeśli tak, a twój kolega się z tym zgadza, to wyraźne stwierdzenie przeciwko temu wydarzeniu powinno być satysfakcjonującym rozwiązaniem dla was obu.


Kompletnie się zgadzam. Jeśli istnieje jakikolwiek przypadek, w którym Twój kod nie jest w stanie obsłużyć, sprawdź dane wejściowe tak wcześnie, jak to możliwe i nie powiedzie się. „Program ulega awarii, jeśli wykonujemy X” jest zawsze lepszy niż „Nasz robot zabija ludzi, jeśli wykonujemy X”
Josef

1
Dobry pomysł. Dobry kod to kod, który udowodnił, że nigdy nie zawodzi, ale jeśli mimo to zawodzi w niewytłumaczalny sposób, zawodzi w dobrze określony sposób, który można naprawić. Kod, który nie może się nie udać, ale jeśli się nie powiedzie, okazuje się niemożliwy do uzyskania lub naprawy, nie jest tak świetny ...
leftarabout około

Zamierzałem opublikować dokładnie to samo. Recenzent wskazuje na możliwą awarię, jak sobie z tym poradzić to inna kwestia.
SH-

2
O nie, nie rób asercji w swoim kodzie. Jeśli aser nie zostanie wkompilowany, nikt go nigdy nie zobaczy. Jeśli aser zostanie wkompilowany, Twój robot się zawiesi. Widziałem więcej niż jeden przypadek, w którym w kodzie produkcyjnym pojawiło się stwierdzenie „coś, co nigdy nie mogło się wydarzyć”, które zniszczyło nie tylko ten system, ale wszystko, co od niego zależało.
Tom Tanner

@TomTanner „z dobrą obsługą awarii, która powinna już być na miejscu”. Zakładam, że kod jest już w stanie obsłużyć niepomyślne stwierdzenia. To może nie być zbyt trudne, ponieważ strategie bezpiecznych awarii powinny być częścią każdego systemu fizycznego.
WorldSEnder

13

Ponieważ wydaje się, że jesteś tam nowy, możesz zrobić tylko jedną rzecz - skonsultować się z kierownikiem zespołu (lub kierownikiem projektu). 13 godzin to decyzja biznesowa; dla niektórych firm / zespołów bardzo dużo; dla niektórych nic. To jeszcze nie twoja decyzja.

Jeśli trop mówi „zakryj tę skrzynkę”, w porządku; jeśli powie „nie, pieprzyć to”, w porządku - jego decyzja, jego odpowiedzialność.

Jeśli chodzi o recenzje kodu, zrelaksuj się. Zwrócenie zadania raz lub dwa razy jest całkowicie normalne.


7

Nie wydaje mi się, żeby jedna rzecz była skierowana w naturze, chociaż była to rodzaj wychowania w odpowiedzi na @SteveBarnes:

Jakie są konsekwencje niepowodzenia?

W niektórych polach błąd oznacza błąd na stronie internetowej. Niebieskie ekrany komputera i restarty.

Na innych polach to życie lub śmierć - samochód samojezdny zamyka się. Medyczny stymulator przestaje działać. Lub w odpowiedzi Steve'a: rzeczy wybuchają, powodując utratę milionów dolarów.

W tych ekstremach istnieje świat różnic.

To, czy 13 godzin na pokrycie „awarii” jest ostatecznie warte, nie powinno zależeć od ciebie. Powinno to zależeć od kierownictwa i właścicieli. Powinni poczuć większy obraz.

Powinieneś być w stanie zgadnąć, co będzie tego warte. Czy twój robot po prostu zwolni czy zatrzyma się? Zmniejszona wydajność? A może awaria robota spowoduje szkody finansowe? Utrata życia?

Odpowiedź na to pytanie powinna prowadzić do odpowiedzi „czy warto 13 godzin czasu firmowego”. Uwaga: Powiedziałem czas firm. Płacą rachunki i ostatecznie decydują, co jest tego warte. Kierownictwo powinno mieć ostatnie słowo w obu przypadkach.


1
Ponadto, jakie są konsekwencje odpowiedzialności za znaną wadę, która nie jest naprawiona, nawet jeśli jest niejasna?
chux

5

Może porozmawiasz z osobą odpowiedzialną za nadanie priorytetu pracy? Podczas uruchamiania może być CTO lub właściciel produktu. Mógłby pomóc ustalić, czy ta dodatkowa praca jest wymagana i dlaczego. Możesz także przynieść swoje zmartwienia podczas codziennych pojedynków (jeśli je masz).

Jeśli nie ma wyraźnej odpowiedzialności (np. Właściciela produktu) za planowanie pracy, spróbuj porozmawiać z ludźmi wokół ciebie. Później może stać się problemem, że wszyscy popychają produkt w przeciwnym kierunku.


5

Najlepszy sposób na rozwiązanie sporu jest taki sam, niezależnie od tego, czy jesteś młodszym programistą, starszym programistą, a nawet dyrektorem generalnym.

Zachowuj się jak Columbo .

Jeśli nigdy nie oglądałeś żadnego Columbo, był to całkiem fantastyczny program. Columbo był tą bardzo skromną postacią - większość ludzi uważała, że ​​jest trochę szalony i nie warto się o niego martwić. Ale wyglądając na pokornego i prosząc ludzi o wyjaśnienia, był w stanie zdobyć swojego człowieka.

Myślę, że ma to również związek z metodą Sokratejską .


Zasadniczo chcesz zadawać pytania sobie i innym, aby upewnić się, że dokonujesz właściwych wyborów. Nie z pozycji „mam rację, mylisz się”, ale z pozycji uczciwego odkrycia. A przynajmniej najlepiej jak potrafisz.

W twoim przypadku masz tutaj dwa pomysły, ale mają one zasadniczo ten sam cel: ulepszenie kodu.

Masz wrażenie, że przeglądanie kodu w potencjalnie nieprawdopodobnym (niemożliwym?) Przypadku na rzecz rozwoju innych funkcji jest najlepszym sposobem na to.

Twój współpracownik ma wrażenie, że bardziej ostrożne podejście do narożnych skrzynek jest cenniejsze.

Co oni widzą, czego nie widzisz? Co widzisz, czego oni nie widzą? Jako młodszy programista masz naprawdę świetną pozycję, ponieważ naturalnie powinieneś zadawać pytania. W innej odpowiedzi ktoś wspomina, jak zaskakująco prawdopodobna jest sprawa narożna. Możesz więc zacząć od: „Pomóż mi zrozumieć - miałem wrażenie, że X, Y i Z - czego mi brakuje? Dlaczego widget zamieni się w ramkę? Miałem wrażenie, że będzie on sflaczały w burzliwych okolicznościach. swizzle stick faktycznie wykuwa pędzle ANZA? ”

Kiedy kwestionujesz swoje założenia i odkrycia, wykopiesz, odkryjesz uprzedzenia, a ostatecznie odkryjesz, jaki jest właściwy sposób działania.

Zacznij od założenia, że ​​wszyscy w Twoim zespole są całkowicie racjonalni i mają na względzie dobro zespołu i produktu, tak jak Ty. Jeśli robią coś, co nie ma sensu, musisz dowiedzieć się, czego nie wiesz, co robią lub co wiesz, że nie.


3

13 godzin to nic wielkiego, po prostu bym to zrobił. Pamiętaj, że zarabiasz za to. Po prostu określ to jako „bezpieczeństwo pracy”. Najlepiej też zachować dobrą karmę w zespole. Teraz, jeśli zajęło ci to tydzień lub dłużej, możesz zaangażować swojego menedżera i zapytać go, czy to najlepszy sposób wykorzystania twojego czasu, zwłaszcza jeśli się z tym nie zgadzasz.

Jednak brzmisz tak, jakbyś potrzebował dźwigni w swojej grupie. Oto jak uzyskać efekt dźwigni: prosić o wybaczenie, nie prosić o pozwolenie. Dodawaj rzeczy do programu według własnego uznania (w zakresie kursu, tj. Upewnij się, że całkowicie rozwiązuje problem, którego szef chce ...), i poinformuj kierownika lub kolegów po fakcie. Nie pytaj ich: „Czy mogę dodawać funkcję X”? Zamiast tego po prostu dodaj funkcje, które osobiście chcesz w programie. Jeśli denerwują się z powodu nowej funkcji lub się z nią nie zgadzają, możesz ją usunąć. Jeśli im się podoba, zatrzymaj to.

Ponadto, ilekroć proszą cię o zrobienie czegoś, przejdź „dodatkową milę” i dodaj wiele rzeczy, o których zapomnieli wspomnieć lub rzeczy, które działałyby lepiej niż to, co powiedzieli. Ale nie pytaj ich, czy „w porządku” pójść o krok dalej. Po prostu zrób to i niechlujnie powiedz im o tym po zakończeniu. Trenujesz ich ...

To, co się stanie, sprawi, że Twój menedżer będzie Cię określał jako „go-getter” i zacznie pokładać w tobie zaufanie, a twoi koledzy zaczną cię postrzegać jako lidera, od którego zaczynasz mieć program. A potem, gdy rzeczy, o których wspomniasz, zdarzą się w przyszłości, będziesz miał więcej do powiedzenia, ponieważ jesteś w zasadzie gwiazdą drużyny, a członkowie drużyny wycofają się, jeśli się z nimi nie zgodzisz.


8
Chociaż jestem za programistami, którzy są proaktywni, a nie tylko „przyjmują zamówienia z góry”, sposób, w jaki to prezentujesz, jest zawodowo nieodpowiedzialny i nieetyczny. Mówisz po prostu, że OP powinien poświęcić czas i pieniądze pracodawcy, nie pracując nad żądanymi funkcjami, ale zamiast tego pracując nad funkcjami, których „osobiście chce”, a następnie poświęcić czas i pieniądze pracodawcy na usunięcie tych funkcji. Nie uwzględnia to nawet potencjalnych wad, które zostaną dodane, ani czasu innego programisty na sprawdzenie / utrzymanie kodu. Zwolniłbym programistę z postawą, którą opisujesz, szczególnie młodszą.
Derek Elkins

1
Cóż, masz rację. Zwłaszcza, jeśli inżynier wyrusza sam, bez żadnej wrażliwości na wizję klienta. Ale nie sabotuj tego, co powiedziałem całkowicie - po prostu powiedziałem, żebym poszedł dalej. Oznacza to, że bierzesz to, co mówi twój szef, i rozwijasz to. A w oprogramowaniu jest to różnica między oprogramowaniem, które wygląda na trywialne gówno, a oprogramowaniem, które wygląda jak stworzone przez profesjonalistę. Znam wielu programistów, którzy robią „dokładnie to, co im powiedziano”, nawet jeśli to, co im powiedziano, jest kompletnym śmieciem. Ci programiści nigdy nie są niczym.

2
Istnieją odpowiedzialne sposoby robienia tego, co opisujesz. Wiele razy wymagania pozostawiają dużo miejsca, a skorzystanie z własnego osądu w celu uzyskania bardziej dopracowanego rezultatu (w porównaniu do wysiłku, w tym wysiłku utrzymania, aby go osiągnąć) jest dobrą rzeczą. Aktywne wskazywanie i naprawianie błędów jest zwykle w porządku. Poświęcenie czasu na stworzenie prototypu funkcji, która Twoim zdaniem leży w interesie firmy, i przedstawienie jej zespołowi w celu ewentualnego włączenia jest również w porządku. Twoje „osobiste potrzeby” nie mają znaczenia, a wynagrodzenie powinno koncentrować się na interesach firmy.
Derek Elkins

Zobacz mój drugi komentarz, w jaki sposób przedstawiłbym, jak sądzę, pomysł, który próbujesz uzyskać. Jak powiedziałem, mój problem dotyczy bardziej sposobu, w jaki go przedstawiłeś. Deweloperzy potrzebują pewnej dumy, aby wiedzieć, że mogą podejmować znaczące decyzje, ale z pokorą, wiedząc, że (zwykle) nie mają szerokiego obrazu celów i priorytetów firmy. Więcej starszych programistów ma mniejsze szanse na podejmowanie złych decyzji i bardziej prawdopodobne jest, aby wiedzieć, jakie są cele firmy i jak się do nich zbliżyć.
Derek Elkins

zauważ również, że mój komentarz dotyczy tych, którzy chcą przejść na poziom wiodący lub konsultant. Firmy zatrudniają mnie specjalnie za moją opinię.

3

Przegląd kodu służy kilku celom. Jedno, o czym oczywiście wiesz, to: „ Czy ten kod jest odpowiedni do celu? ” Innymi słowy, czy jest funkcjonalnie poprawny; czy jest odpowiednio przetestowany; czy nieoczywiste części zostały odpowiednio skomentowane; czy jest zgodny z konwencjami projektu?

Kolejną częścią przeglądu kodu jest dzielenie się wiedzą o systemie. Jest to okazja zarówno dla autora, jak i recenzenta, aby dowiedzieć się o zmienionym kodzie i jego interakcji z resztą systemu.

Trzecim aspektem jest to, że może zapewnić przegląd problemów, które istniały przed wprowadzeniem jakichkolwiek zmian. Dość często, gdy przeglądam czyjeś zmiany, zauważam coś, co przeoczyłem w poprzedniej iteracji (dość często coś własnego). Stwierdzenie typu „Oto okazja, aby uczynić to bardziej solidnym niż było”, nie jest krytyką i nie traktuj tego jako jednego!

Mój obecny zespół traktuje przegląd kodu nie tylko jako bramę lub przeszkodę, którą kod musi przejść bez szwanku przed zatwierdzeniem, ale przede wszystkim jako okazję do nieco uporządkowanej dyskusji na temat określonego obszaru funkcjonalności. To jedna z najbardziej produktywnych okazji do wymiany informacji. (I to dobry powód, aby podzielić się recenzją w zespole, a nie zawsze z tym samym recenzentem).

Jeśli postrzegasz recenzje kodu jako działanie konfrontacyjne, jest to samospełniająca się przepowiednia. Jeśli zamiast tego uważasz je za najbardziej współpracującą część pracy, będą one stymulować ciągłe doskonalenie Twojego produktu i sposobu współpracy. Pomaga, jeśli recenzja może jasno określić względne priorytety swoich sugestii - na przykład istnieje różnica między „Chciałbym tu przydatnego komentarza” a „To łamie się, jeśli xjest zawsze negatywne”.

Po złożeniu wielu ogólnych stwierdzeń powyżej, jak to się ma do twojej konkretnej sytuacji? Mam nadzieję, że teraz jest oczywiste, że moją radą jest odpowiedzieć na recenzję otwartymi pytaniami i wynegocjować, które podejście ma największą wartość. Na przykład w przypadku, gdy sugerowany jest dodatkowy test, to coś w stylu: „Tak, możemy to przetestować; szacuję, że wdrożenie zajmie <czas> . Czy uważasz, że korzyść jest tego warta? Czy jest coś jeszcze można zrobić, aby zagwarantować, że test nie jest konieczny? ”


Jedna rzecz, która mnie uderza, gdy czytam twoje pytanie: jeśli napisanie nowego przypadku testowego zajmuje dwa dni, to albo nowy test jest zupełnie innym scenariuszem niż istniejące testy (w takim przypadku prawdopodobnie ma dużo wartość) lub stwierdziłeś potrzebę lepszego ponownego wykorzystania kodu w pakiecie testowym.


Wreszcie, jako ogólny komentarz na temat wartości recenzji kodu (i jako zwięzłe podsumowanie tego, co powiedziałem powyżej), podoba mi się to oświadczenie, w Maintainers Don't Scale autorstwa Daniela Vettera :

Przynajmniej dla mnie przegląd nie polega tylko na zapewnieniu dobrej jakości kodu, ale także na rozpowszechnianiu wiedzy i poprawie zrozumienia. Na początku może jedna osoba, autor (i to nie jest dane), rozumie kod. Po dobrej recenzji powinny być co najmniej dwie osoby, które w pełni to rozumieją, w tym przypadki narożne.


3

Kod może ZAWSZE być lepszy.

Jeśli przeglądasz kod i nie widzisz niczego, co mogłoby być lepsze lub testu jednostkowego, który mógłby wykryć błąd, to nie jest to kod idealny, ale recenzent, który nie wykonuje swojej pracy. To, czy zdecydujesz się wspomnieć o usprawnieniu, jest osobistym wyborem. Ale prawie za każdym razem, gdy Twój zespół dokonuje przeglądu kodu, powinny być rzeczy, które ktoś zauważy, które mogą być lepsze lub wszyscy prawdopodobnie zmarnują swój czas.

To, czy zdecydujesz się na komentarze, czy nie, zależy od Twojego zespołu. Jeśli Twoje zmiany naprawią problem lub dodadzą wystarczającej wartości bez zmian, które Twój zespół akceptuje, to połącz je i zaloguj swoje komentarze do rejestru, aby ktoś mógł się później zająć. Jeśli zespół stwierdzi, że twoje zmiany powodują większe ryzyko lub złożoność niż wartość, musisz odpowiednio rozwiązać problemy.

Pamiętaj tylko, że każdy kod ma co najmniej jeszcze jeden przypadek brzegowy, który można przetestować i który może użyć co najmniej jeszcze jednego refaktoryzacji. Właśnie dlatego recenzje kodu najlepiej przeprowadzać jako grupę, w której wszyscy oglądają ten sam kod w tym samym czasie. Aby w końcu wszyscy mogli dojść do konsensusu co do tego, czy przeglądany kod jest akceptowalny (tak jak jest) i wnosi wystarczającą wartość do scalenia z bazą społeczności lub czy pewne rzeczy muszą zostać wykonane, zanim będzie wystarczająca wartość do scalenia .

Ponieważ zadajesz to pytanie, zakładam, że tak naprawdę nie robisz „recenzji kodu”, ale tworzysz żądanie ściągnięcia lub inny mechanizm przesyłania, aby inni mogli komentować w sposób niedeterministyczny. Teraz masz problem z zarządzaniem i definicję ukończenia. Sądzę, że twoje zarządzanie jest niezdecydowane i tak naprawdę nie rozumie procesu i celu recenzji kodu i prawdopodobnie nie ma „definicji ukończenia” (DOD). Ponieważ gdyby to zrobili, twój DOD odpowiedziałby wprost na to pytanie i nie musiałbyś tu przychodzić i pytać.

Jak to naprawić? Cóż, poproś swojego menedżera, aby dał ci DOD i powie ci, czy musisz zawsze wdrażać wszystkie komentarze. Jeśli dana osoba jest Twoim przełożonym, odpowiedź jest oczywista.


3

To pytanie nie dotyczy zalet programowania obronnego, niebezpieczeństw związanych z przypadkami narożnymi ani katastrofalnego ryzyka błędów w produktach fizycznych. W rzeczywistości w ogóle nie chodzi o inżynierię oprogramowania .

Tak naprawdę chodzi o to, jak młodszy praktykujący radzi sobie z instrukcją od starszego praktyka, gdy młodszy nie może się z tym zgodzić lub docenić.

Są dwie rzeczy, które musisz docenić będąc młodszym programistą. Po pierwsze, oznacza to, że chociaż możliwe jest , że masz rację, a on nie ma racji, jest mało prawdopodobne. Jeśli twój współpracownik sugeruje, że nie widzisz wartości, musisz poważnie zastanowić się nad możliwością, że po prostu nie masz wystarczającego doświadczenia, aby to zrozumieć. Nie rozumiem tego postu.

Drugą rzeczą, którą należy docenić, jest to, że twój starszy partner jest tak zwany, ponieważ ma większą odpowiedzialność. Jeśli junior złamie coś ważnego, nie będą mieli kłopotów, jeśli zastosują się do instrukcji. Gdyby jednak senior pozwolił im to zepsuć - nie podnosząc na przykład problemów z przeglądaniem kodu - słusznie mieliby dużo kłopotów.

Ostatecznie bezwzględnym wymogiem Twojej pracy jest przestrzeganie instrukcji od zaufanych firm do prowadzenia ich projektów. Czy generalnie nie jesteś w stanie odłożyć się na emerytów, jeśli istnieje dobry powód, by cenić ich doświadczenie? Czy zamierzasz postępować zgodnie z instrukcjami, których nie rozumiesz? Czy uważasz, że świat powinien się zatrzymać, dopóki nie będziesz przekonany? Wartości te są niezgodne z pracą w zespole.

Ostatni punkt. Wróć do projektów napisanych sześć miesięcy temu. Pomyśl o projektach, które napisałeś na uniwersytecie. Zobacz, jak źle się teraz wydają - wszystkie błędy i odwrócone wzornictwo, martwe punkty i błędne abstrakcje? Co jeśli powiem ci, że za sześć miesięcy policzysz te same wady w pracy, którą dzisiaj wykonujesz? Czy to pomaga pokazać, ile martwych punktów jest w twoim obecnym podejściu? Ponieważ taka jest różnica w doświadczeniu.


2

stale dodaje komentarze do mojego kodu, które sugerują, żebym wykonał o wiele więcej pracy niż jest to konieczne.

Możesz dawać rekomendacje, ale ostatecznie to nie twoja rola decyduje, co jest konieczne. Twoim zadaniem jest wdrożyć to, co zarząd lub (w tym przypadku twój recenzent) uzna za konieczne. Jeśli nie zgadzasz się z tym, co jest konieczne zbyt mocno lub zbyt mocno, prawdopodobnie nie znajdziesz pracy. W związku z tym częścią twojego profesjonalizmu jest pogodzenie się z tym i zachowanie pokoju.

Zasugerował, że zajmę się niejasną sprawą, która jest bardzo mało prawdopodobna

Są tutaj inne świetne odpowiedzi, które pokazują, że nawet nie-błędy (tj. Coś, co może nigdy nie zawieść ) nadal powinny być czasem przerabiane. (np. w przypadku budowania w przyszłości bezpieczeństwa produktu, przestrzegania standardów itp.) Wielką rolę programisty polega na tym, aby mieć jak największą pewność, że Twój kod będzie niezawodny w każdej możliwej sytuacji za każdym razem i w przyszłości - również zabezpieczony, nie tylko pracujący w testowanych sytuacjach w obecnych warunkach przez większość czasu


2

Sugestie dla recenzentów kodu, aby zwiększyć użyteczność biznesową recenzji kodu (Ty jako OP powinieneś zaproponować taką zmianę):

  • Oznacz swoje komentarze według rodzaju. „Krytyczny” / „Obowiązkowy” / „Opcjonalny” / „Sugerowane ulepszenia” / „miło mieć” / „Rozmyślam”.

    Jeśli wydaje się to zbyt CDO / analne / skomplikowane, użyj co najmniej 2 poziomów: „Musisz naprawić, aby przejść przegląd i mieć możliwość scalenia zmiany” / „Wszystkie inne”.

Sugestie dotyczące obsługi sugestii przeglądu kodu, które wydają się mniej istotne, aby:

  • Stwórz otwarty bilet w wybranym przez siebie systemie biletowym (twój zespół ma go, mam nadzieję?), Śledząc sugestię

  • Umieść bilet # jako komentarz odpowiedzi do pozycji przeglądu kodu, jeśli twój proces pozwala na odpowiedzi na komentarze takie jak Rybie oko lub recenzje e-mailem.

  • Skontaktuj się z recenzentem i wyraźnie zapytaj, czy ten element jest typu „należy naprawić, czy nie zostanie scalony / wydany”.

    • Jeśli odpowiedź brzmi „Tak”, ale nie zgadzasz się, pozwól osobie odpowiedzialnej za zarządzanie projektem (PM, kierownik, itp.) Podjąć decyzję - przedstaw w pełni i uczciwie nieporozumienie. Nie chodzi o to, które z was jest „właściwe”, ale o to, co jest lepsze dla projektu, więc praca PM / kierownika.

Teraz traktuj ten bilet jak każde inne żądanie programowania.

  1. Jeśli po eskalacji postanowiono, że będzie to pilne, potraktuj to jak każdą pilną prośbę dewelopera. Zdepreoretuj inne prace i pracuj nad tym.

  2. W przeciwnym razie pracuj nad nim zgodnie z priorytetem, który został przypisany i jego ROI (które mogą się różnić w zależności od linii biznesowej, jak wyjaśniono w innych odpowiedziach).


2

Należy nie eskalować to do zarządzania.

W większości firm menedżerowie zawsze decydują się nie pisać tego dodatkowego testu, nie tracić czasu na nieznaczną poprawę jakości kodu, aby nie tracić czasu na refaktoryzację.

W wielu przypadkach jakość kodu zależy od niepisanych reguł w zespole programistów i dodatkowego wysiłku włożonego przez programistów.

Jesteś młodszym programistą i jest to Twoja pierwsza recenzja kodu . Musisz zaakceptować radę i wykonać pracę. Możesz usprawnić przepływ pracy i zasady w zespole tylko wtedy, gdy najpierw je znasz i szanujesz, aby zrozumieć, dlaczego one istnieją. W przeciwnym razie będziesz nowym facetem, który nie przestrzega zasad i staje się samotnym wilkiem w drużynie.

Jesteś nowy w zespole, postępuj zgodnie ze wskazówkami, które otrzymujesz, dowiedz się, dlaczego one tam są, nie podawaj pierwszych wskazówek, które można zakwestionować na spotkaniu scrumowym. Rzeczywiste priorytety biznesowe staną się dla ciebie oczywiste po pewnym czasie bez pytania (i może nie być tym, co facet od zarządzania powie ci osobiście).


Niestety, kiedy dobrze zacząłeś, twoja rekomendacja okazuje się dość zła.
Joshua

1

Bardzo ważne jest, abyś tworzył kod, który spełnia wymagania twojego żądania wiodącego / zarządzania. Każdy inny szczegół byłby po prostu „miło mieć”. Jeśli jesteś ekspertem (czytaj: „jeśli nie jesteś młodszym programistą”) w swojej dziedzinie, to jesteś „uprawniony” do rozwiązywania drobnych problemów, które napotkasz po drodze, bez konsultacji z liderami za każdym razem.

Jeśli uważasz, że coś jest nie tak i jesteś względnie ekspertem w swojej dziedzinie, masz duże szanse, że masz rację .

Oto kilka stwierdzeń, które mogą Ci się przydać:

  • Poproszono mnie o zrobienie X, recenzent kodu sugeruje również wykonanie Y, czy powinienem to zrobić, czy powinienem przejść do ważniejszych rzeczy?
  • Sugerujesz mi Y, więc czy możesz wymyślić co najmniej jeden przypadek testowy, który wychwyciłby to zachowanie, abym mógł je przetestować? Wierzę, że ten kod nie zostanie wywołany.
  • Może nie powinniśmy najpierw opracować bezpiecznego rozwiązania awaryjnego dla odkrytych przypadków? Więc łapiemy je wcześnie i naprawiamy w ruchu, abyśmy mogli skupić się na ważniejszych rzeczach.
  • OK, kiedy wdrażam Y, czy możesz być tak uprzejmy, aby napisać dla niego kilka przypadków testowych, abyśmy załatwili sprawę raz na zawsze ?
  • Poproszono mnie o wykonanie X i myślę, że mógłbym wykonać Y, chyba że istnieją inne priorytety. Następnym razem, dlaczego nie zgłosisz żądania funkcji zamiast wstawić go jako komentarz do recenzji mojego kodu ? Przynajmniej możemy usłyszeć dalsze opinie innych członków zespołu na temat tej funkcji przed jej wdrożeniem (generalnie wszelkie ważne rzeczy powinny być cechą i powinny być obsługiwane przez więcej niż jedną osobę; zwykle przegląd kodu powinien dotyczyć przeglądu kodu i rozwiązań rozwiązań; reszta naprawia błędy i funkcje).

Czy poważnie myślisz, że postawa recenzenta szkodzi projektowi, czy też uważasz, że ma on większość czasu (chyba że czasami popełnił drobne błędy w ocenie i przesadził)?

Dla mnie brzmi to bardziej, jak pisze rzeczy, które nie należą do recenzji kodu, co jest złą praktyką, ponieważ utrudnia wszystkim śledzenie rzeczy. Nie wiem jednak, jakie inne komentarze do recenzji napisał, więc nie mogę nic powiedzieć o innych sprawach.

Ogólnie staraj się unikać obu następujących elementów:

  • Nie robisz tego, o co prosiłeś
  • Sprawiasz, że twój recenzent jest głupi

Jeśli rzeczywiście sprawdza Twój kod, to dlatego, że kierownictwo ufa mu bardziej niż tobie.


-1

Jeśli podczas przeglądu kodu nie ma zgody co do zakresu:

  1. Dokumentuj zakres, który jest faktycznie objęty kodem. Nikt nie lubi przykrych niespodzianek.
  2. Zrozum, że zakres jest decyzją biznesową. Zakres powinien być już znany przed rozpoczęciem pracy nad funkcją, ale jeśli nie, zawsze możesz poprosić o wyjaśnienie później.

Jeśli recenzent kodu podejmuje decyzje biznesowe, może zmienić zakres w dowolnym momencie, nawet podczas przeglądania kodu, ale nie robi tego w roli recenzenta kodu.


wydaje się, że nie oferuje to nic istotnego w porównaniu z punktami przedstawionymi i wyjaśnionymi w poprzednich 20 odpowiedziach
komnata

-1

Jeśli nie możesz udowodnić, że przypadek na krawędzi jest niemożliwy, musisz założyć, że jest to możliwe. Jeśli jest to możliwe, nieuniknione jest, że w końcu to nastąpi, a raczej wcześniej niż później. Jeśli przypadek krawędzi nie wystąpił podczas testowania, może to wskazywać, że zasięg testu jest niepełny.

  1. Zaakceptuj informacje zwrotne.
  2. Przed wprowadzeniem jakichkolwiek zmian w kodzie postaraj się zbudować test dla przypadku krawędzi i sprawdź, czy możesz uzyskać błąd testu (dowód na istnienie problemu). Jeśli niemożliwe jest utworzenie takiego przypadku testowego i uzyskanie błędu testowego, możesz dojść do wniosku, że przypadek skrajny jest w rzeczywistości niemożliwy (chociaż wahałbym się wyciągnąć taki wniosek).
  3. Jeśli możesz uzyskać błąd testu, zastosuj odpowiednią zmianę kodu, aby przejść test.

Dla dobra produktu prawdopodobnie prawdopodobnie będziesz w stanie wyprodukować obudowę krawędzi i wywołać awarię, abyś mógł zastosować poprawkę i mieć pewność, że potencjalny problem został rozwiązany.

Głównym celem recenzji kodu jest dodatkowe spojrzenie na kod. Nikt z nas nie jest odporny na błędy i niedopatrzenia. To zbyt często spoglądać na fragment kodu wiele razy i nie zauważać oczywistego błędu, w którym świeża para oczu może go natychmiast odczytać.

Jak powiedziałeś, zaimplementowałeś nowy algorytm. Rozsądne byłoby wyciąganie wniosków na temat zachowania nowego algorytmu na podstawie zachowania lub obserwacji jego poprzednika.


wydaje się, że nie oferuje to nic istotnego w porównaniu z punktami przedstawionymi i wyjaśnionymi w poprzednich 21 odpowiedziach
komnata

-2

Są recenzenci kodu, którzy wiedzą, co robią, a jeśli twierdzą, że coś musi zostać zmienione, to trzeba to zmienić, a kiedy mówią, że coś musi zostać przetestowane, to musi zostać przetestowane.

Są recenzenci kodu, którzy muszą uzasadnić swoje istnienie, tworząc niepotrzebną pracę dla innych.

Który z nich należy do ciebie, i jak poradzić sobie z drugim rodzajem, jest bardziej kwestią dla miejsca pracy. Wymiana stosów.

Jeśli używasz scrum, to pytanie brzmi, czy twoja praca wykonuje to, co powinna (najwyraźniej tak), i możesz umieścić obsługę bardzo rzadkiego i być może niemożliwego przypadku na zaległości, gdzie będzie on traktowany priorytetowo, i czy przechodzi do sprintu, a następnie recenzent może go odebrać i wykonać 13 godzin pracy. Jeśli wykonujesz zadanie X, a ponieważ wykonujesz zadanie X, zdajesz sobie sprawę, że zadanie Y również musi zostać wykonane, to zadanie Y nie staje się częścią zadania X, jest to jego samodzielna praca.


6
Jest to zbyt niejasne i emocjonalne, aby było przydatne.
Robert Harvey

Uwaga o SCRUM jest całkowicie słuszna, juts tworzą nowe zadanie w zaległościach. Usuń niegrzeczny początek swojej odpowiedzi, a otrzymasz pozytywny wynik.
xmedeko
Korzystając z naszej strony potwierdzasz, że przeczytałeś(-aś) i rozumiesz nasze zasady używania plików cookie i zasady ochrony prywatności.
Licensed under cc by-sa 3.0 with attribution required.