Pod koniec 2-tygodniowego sprintu, a zadanie ma przegląd kodu, podczas przeglądu odkryliśmy funkcję, która działa, jest czytelna, ale jest dość długa i ma kilka zapachów kodu. Łatwa praca z refaktorem.
W przeciwnym razie zadanie pasuje do definicji gotowego.
Mamy dwie możliwości.
- Niepowodzenie przeglądu kodu, aby bilet nie zamykał się w tym sprincie, i odbieramy trochę morale, ponieważ nie możemy przekazać biletu.
- Refaktor to niewielki kawałek pracy, który zostałby wykonany w następnym sprincie (lub nawet przed jego rozpoczęciem) jako mała, półpunktowa historia.
Moje pytanie brzmi: czy istnieją jakieś nieodłączne problemy lub względy związane z podniesieniem biletu z tyłu recenzji, zamiast go zawieść?
Zasoby, które mogę znaleźć i przeczytać szczegółowe recenzje kodu jako 100% lub nic, zwykle, ale uważam, że to zwykle nie jest realistyczne.
agile
code-reviews
Erdrik Ironrose
źródło
źródło
Odpowiedzi:
Nie z natury. Na przykład implementacja bieżącej zmiany mogła odkryć problem, który już istniał, ale do tej pory nie był znany / widoczny. Niepowodzenie biletu byłoby niesprawiedliwe, ponieważ zawiedziesz go za coś niezwiązanego z faktycznie opisanym zadaniem.
Jednak przypuszczam, że ta funkcja została dodana przez obecną zmianę. W takim przypadku bilet powinien się nie powieść, ponieważ kod nie przeszedł testu węchu.
Gdzie narysujesz linię, jeśli nie w miejscu, w którym już ją narysowałeś? Najwyraźniej nie uważasz, że ten kod jest wystarczająco czysty, aby pozostać w bazie kodu w jego obecnej formie; dlaczego więc miałbyś rozważyć podanie biletu?
Wydaje mi się, że pośrednio argumentujesz, że próbujesz dać temu biletowi przepustkę, by zwiększyć morale zespołu, a nie jakość bazy kodowej.
Jeśli tak, to masz różne priorytety. Standard czystego kodu nie powinien być zmieniany tylko dlatego, że sprawia, że zespół jest szczęśliwszy. Poprawność i czystość kodu nie zależy od nastroju zespołu.
Jeśli implementacja oryginalnego biletu spowodowała zapach kodu, należy go rozwiązać w oryginalnym bilecie. Powinieneś tworzyć nowy bilet tylko wtedy, gdy zapach kodu nie może być bezpośrednio przypisany do oryginalnego biletu (na przykład scenariusz „słomy, która złamała grzbiet wielbłąda”).
Pass / fail jest z natury stanem binarnym , który z natury jest wszystkim lub niczym.
Myślę, że to, o czym tu mówisz, polega na tym, że interpretujesz recenzje kodu jako wymagające doskonałego kodu lub w inny sposób go nie udaje, i tak nie jest.
Kod nie powinien być nieskazitelny, powinien po prostu odpowiadać rozsądnemu standardowi czystości stosowanemu przez Twój zespół / firmę. Przestrzeganie tego standardu jest wyborem binarnym: przestrzega (przechodzi) lub nie (kończy się niepowodzeniem).
Na podstawie opisu problemu jasne jest, że nie sądzisz, że jest on zgodny z oczekiwanym standardem kodu, a zatem nie należy go przekazywać z ukrytych powodów, takich jak morale zespołu.
Gdyby „wykonał zadanie” był najlepszym punktem odniesienia dla jakości kodu, nie musielibyśmy na początku wymyślać zasady czystego kodu i dobrych praktyk - kompilator i testy jednostkowe byłyby już naszym zautomatyzowanym procesem przeglądu i nie potrzebujesz recenzji kodu ani argumentów stylu.
źródło
Dlaczego pojawia się na końcu sprintu? Przegląd kodu powinien nastąpić, gdy tylko pomyślisz, że kod jest gotowy (lub nawet wcześniej). Powinieneś sprawdzić swoją definicję ukończenia z każdą ukończoną historią.
Jeśli kończysz opowiadanie tak krótko przed recenzją demo / sprintu, że nie możesz tego zrobić jako „małego” zadania, musisz lepiej oszacować swoją pracę. Tak, ta historia nie została ukończona. Nie z powodu przeglądu kodu, ale dlatego, że nie planowałeś wprowadzić zmian z przeglądu kodu. To tak, jakby oszacować, że „testowanie” zajmie zero czasu, ponieważ „jeśli zaprogramujesz go poprawnie, po prostu zadziała, prawda?”. To nie tak działa. Testy wykryją błędy, a przegląd kodu znajdzie rzeczy do zmiany. Gdyby tak nie było, byłoby to stratą czasu.
Podsumowując: tak, DoD jest binarny. Zdać lub oblać. Przegląd kodu nie jest binarny, powinien być raczej ciągłym zadaniem. Nie możesz zawieść . To proces, który ostatecznie się zakończył. Ale jeśli nie planujesz odpowiednio, nie dojdziesz do tego „ukończonego” etapu na czas i utkniesz na terytorium „niezrealizowanym” na końcu sprintu. To nie jest dobre dla morale, ale musisz wziąć to pod uwagę w planowaniu.
źródło
Proste: przeglądasz zmianę . W innym przypadku nie sprawdzasz stanu programu. Jeśli naprawię błąd w funkcji linii 3000, to sprawdzisz, czy moje zmiany naprawiają błąd i to wszystko. A jeśli moja zmiana naprawi błąd, akceptujesz zmianę.
Jeśli uważasz, że funkcja jest za długa, wówczas wysyłasz żądanie zmiany, aby ją skrócić lub podzielić, po zaakceptowaniu mojej zmiany, a następnie żądanie zmiany może zostać uszeregowane według ważności. Jeśli zostanie podjęta decyzja, że zespół ma ważniejsze rzeczy do zrobienia, zostanie to rozwiązane później.
Byłoby śmieszne, gdybyś mógł zdecydować o priorytetach rozwoju podczas przeglądu kodu, a odrzucenie mojej zmiany z tego powodu byłoby próbą ustalenia priorytetów rozwoju.
Podsumowując, absolutnie dopuszczalne jest zaakceptowanie zmiany kodu i natychmiastowe podniesienie biletu na podstawie rzeczy, które widziałeś podczas przeglądania zmiany. W niektórych przypadkach zrobisz to nawet, jeśli sama zmiana spowodowała problemy: Jeśli ważniejsze jest, aby zmiany były teraz, niż naprawić problemy. Na przykład, jeśli inni zostali zablokowani, czekając na zmianę, to chcesz je odblokować, a kod można ulepszyć.
źródło
To wydaje się być problemem.
Teoretycznie wiesz, co powinieneś zrobić, ale zbliża się termin, więc nie chcesz robić tego, co powinieneś zrobić.
Odpowiedź jest prosta: rób wszystko, co byś zrobił, gdybyś otrzymał ten sam kod do przeglądu kodu pierwszego dnia sprintu. Jeśli byłoby to do przyjęcia, to powinno teraz. Gdyby nie to, nie byłoby teraz.
źródło
Ogromna część tego procesu polega na podejmowaniu decyzji, co się stanie , i trzymaniu się broni. Oznacza to również, że nie trzeba nadmiernie angażować się i przeprowadzać przeglądów na czas, aby testy pozwoliły upewnić się, że praca jest również funkcjonalnie ukończona.
Jeśli chodzi o problemy z przeglądaniem kodu, istnieje kilka sposobów, aby sobie z tym poradzić, a właściwy wybór zależy od kilku czynników.
Najważniejsze jest to, że kiedy skończysz pracę, musisz ją wykonać. Jeśli występują problemy większe niż te, nad którymi pracował programista, podnieś flagę i przejdź dalej. Ale nie powinieneś być w pozycji, w której są godziny przed końcem sprintu, a teraz możesz przejść do przeglądu. Pachnie to nadmiernym zaangażowaniem zasobów lub zwlekaniem z recenzjami. (zapach procesu).
źródło
Nie ma żadnych nieodłącznych problemów związanych z depriorytetyzacją problemów związanych z przeglądem kodu, ale wydaje się, że głównymi kwestiami, na które musisz się zgodzić jako zespół, są:
Wszystko sprowadza się do tego, co zespół zgodził się na definicję Gotowe. Jeśli przekazanie przeglądu kodu przy zerowych problemach jest definicją wykonaną dla elementu pracy, nie można zamknąć elementu, który nie spełnił tego wymagania.
To tak, jakby podczas testów jednostkowych test jednostkowy nie powiódł się. Naprawiłbyś błąd, nie ignorując testu jednostkowego, gdyby zaliczenie testów jednostkowych było warunkiem ukończenia.
Jeśli zespół nie zgodził się, aby recenzje kodu były definicją Gotowe, recenzje kodu nie są testem akceptacji bramkowej elementu pracy. Są to działania zespołu, które są częścią procesu zaległości w celu poszukiwania dodatkowej pracy, która może być potrzebna. W takim przypadku wszelkie wykryte problemy nie są związane z wymaganiami oryginalnego elementu pracy i są nowymi elementami pracy dla zespołu, dla których priorytetem jest priorytet.
Na przykład zespół może całkowicie zaakceptować zmianę priorytetu literówek naprawczych w niektórych nazwach zmiennych, ponieważ nie wpływa to na dostarczoną funkcjonalność biznesową, nawet jeśli zespół naprawdę nie lubi patrzeć na nazwę zmiennej „myObkect”.
źródło
Tutaj głosowane odpowiedzi są bardzo dobre; ten dotyczy kąta refaktoryzacji.
W większości przypadków większość pracy przy refaktoryzacji polega na zrozumieniu istniejącego kodu; zmiana go później jest zazwyczaj mniejszą częścią pracy z jednego z dwóch powodów:
Jeśli tylko uczynisz kod bardziej przejrzystym i / lub zwięzłym, niezbędne zmiany są oczywiste. Często zrozumiałeś kod, wypróbowując zmiany, które wydawały się czystsze i sprawdzając, czy rzeczywiście zadziałały, czy też pominęły subtelność w bardziej złożonym kodzie.
Masz już na myśli konkretny projekt lub konstrukcję, której potrzebujesz, aby ułatwić budowę nowej funkcji. W takim przypadku praca nad opracowaniem tego projektu była częścią historii, która spowodowała jego potrzebę; jest to niezależne od tego, czy musisz dokonać refaktoryzacji, aby dostać się do tego projektu.
Uczenie się i rozumienie istniejącego kodu to niezła ilość pracy dla nietrwałej korzyści (za miesiąc prawdopodobnie ktoś dużo zapomniał o kodzie, jeśli w tym czasie nie będzie go czytał ani z nim nie pracował), więc nie ma sensu tego robić, z wyjątkiem obszarów kodu, które powodują problemy lub które planujesz zmienić w najbliższej przyszłości. Z kolei, ponieważ jest to główna praca refaktoryzacji, nie powinieneś refaktoryzować kodu, chyba że powoduje to obecnie problemy lub planujesz go zmienić w najbliższej przyszłości.
Jest jednak jeden wyjątek: jeśli ktoś obecnie dobrze rozumie kod, który z czasem wycieknie, użycie tego zrozumienia, aby kod był bardziej przejrzysty i szybciej zrozumiany, może być dobrą inwestycją. W takiej sytuacji znajduje się ktoś, kto właśnie skończył pisać historię.
W tym przypadku, że myślisz o stworzeniu osobnej historii do refaktoryzacji, jest to znak ostrzegawczy z kilku stron:
Nie myślisz o refaktoryzacji jako o części kodowania, ale o osobnej operacji, która z kolei może spowodować upadek pod presją.
Tworzysz kod, który będzie wymagał więcej pracy, aby zrozumieć następnym razem, gdy ktoś będzie musiał z nim pracować, dzięki czemu opowiadania będą trwać dłużej.
Być może marnujesz czas i energię na refaktoryzację rzeczy, z których nie czerpiesz dużych korzyści. (Jeśli zmiana nastąpi znacznie później, ktoś nadal będzie musiał ponownie zrozumieć kod; i tak jest to bardziej efektywnie połączone z zadaniem refaktoryzacji. Jeśli zmiana nie nastąpi później, refaktoryzacja nie w ogóle cel, może z wyjątkiem estetycznego).
Tak więc odpowiedzią jest, że element nie powiedzie się, aby wyjaśnić, że coś w twoim procesie nie powiodło się (w tym przypadku jest to programista lub zespół, który nie przeznacza czasu na przegląd i wdrażanie zmian, które pochodzą z przeglądu) i każe deweloperowi natychmiast kontynuować pracę na przedmiocie.
Kiedy przejdziesz do oszacowania dla następnej iteracji, ponownie oszacuj istniejącą historię, ponieważ wydaje się, że pozostało tyle pracy, aby przejść przegląd i dodać ją do następnej iteracji, ale zachowując oszacowanie z poprzedniej iteracji. Kiedy historia zostanie ukończona pod koniec następnej iteracji, ustaw historyczną całkowitą ilość pracy na sumę pierwszego i drugiego oszacowania, abyś wiedział, ile naprawdę oszacowano pracy. Pomoże to w uzyskaniu dokładniejszych szacunków podobnych historii w przyszłości przy obecnym stanie procesu. (Tj. Nie zakładaj, że twoje niedoszacowanie nie powtórzy się; załóż, że to się powtórzy, dopóki nie ukończysz podobnych opowiadań przy mniejszym nakładzie pracy).
źródło
Dziwi mnie brak odpowiedzi w odpowiedziach i komentarzach na pojęcie „nieudanego” przeglądu kodu, ponieważ nie jest to pojęcie, które osobiście znam. Nie czułbym się też komfortowo z tym konceptem ani z nikim w moim zespole, który używałby tej terminologii.
Twoje pytanie wyraźnie dotyczy „zwinnych praktyk”, więc wróćmy do manifestu zwinnego (moje podkreślenie):
Porozmawiaj ze swoim zespołem. Omów dany kod. Oceń koszty i korzyści i zdecyduj - jako spójna grupa ekspertów - czy zmienić kod tego kodu teraz, później, czy nigdy.
Rozpocznij współpracę. Przestań nieudanych recenzji kodu.
źródło
w recenzji odkryliśmy funkcję, która działa, jest czytelna, ale jest dość długa i ma kilka zapachów kodu ...
Czy są jakieś nieodłączne problemy lub względy związane z podniesieniem biletu z tyłu recenzji, zamiast go zawieść?
Żaden problem (moim zdaniem zespołów). Zakładam, że kod spełnia kryteria akceptacji określone na bilecie (tzn. Działa). Utwórz element zaległości, aby uwzględnić długość i każdy zapach pachnie, i nadaj mu priorytet, tak jak każdy inny bilet. Jeśli naprawdę jest mały, po prostu nadaj mu priorytet na następny sprint.
Jednym z powiedzeń, które mamy, jest „Wybierz postępową poprawę w stosunku do opóźnionej doskonałości”.
Mamy bardzo płynny proces i zbudowaliśmy dość dużą liczbę funkcji „proof of concept” (1 lub 2 na sprint), które przechodzą przez programistę i testy, ale nigdy nie przechodzą przez wewnętrzny przegląd interesariuszy (hmm, możemy to zrobić zamiast tego ?), alfa lub beta ... niektórzy przeżyją, inni nie.
W bieżącym projekcie straciłem kontrolę nad tym, ile razy zbudowaliśmy określoną funkcję, dostałem ją w ręce interesariuszy, a sprint lub dwa później całkowicie ją usunąłem, ponieważ zmienił się kierunek produktu lub wymagania pełne przekształcenie sposobu wdrożenia tej funkcji. Wszelkie pozostałe zadania „udoskonalania” dla usuniętej funkcji lub niespełniające nowych wymagań zostaną usunięte, jak również część zalegania zaległości.
źródło
Moim zdaniem istnieją dwa sposoby spojrzenia na ten problem:
Z naukowego punktu widzenia większość procesów sprawdzania kodu nie udaje się wdrożyć PBI (element rejestru produktu), gdy nie jest spełniony standard jakości kodu.
Jednak nikt w prawdziwym świecie nie podąża zwinnie do T, ponieważ z jednego (z wielu powodów) różnych branż mają różne wymagania. W związku z tym ustalenie kodu teraz lub zaciągnięcie długu technicznego (najprawdopodobniej utworzyłbyś nowy PBI) powinno być podejmowane indywidualnie dla każdego przypadku. Jeśli ma to narazić na szwank sprint lub zwolnienie lub wprowadzić nieuzasadnione ryzyko, w podejmowanie decyzji powinni wziąć udział interesariusze.
źródło
Ani . Jeśli przegląd kodu nie powiedzie się, zadanie nie zostanie wykonane. Ale nie można zawieść recenzji kodu na podstawie osobistej opinii. Kod przechodzi; przejdź do następnego zadania.
Powinno to być łatwe wywołanie, a fakt, że nie jest to sugerowane, że nie masz wystarczająco jasnych zapisanych reguł dla recenzji kodu.
„Funkcja jest dość długa”. Zapisz: Funkcje muszą mieć mniej niż X linii ( nie sugeruję, że zasady dotyczące długości funkcji są dobre).
„Jest kilka zapachów kodu”. Zapisz: funkcje publiczne muszą mieć testy jednostkowe pod kątem funkcjonalności i wydajności, zarówno użycie procesora, jak i pamięci musi mieścić się w granicach xiy.
Jeśli nie potrafisz określić ilościowo zasad przekazywania recenzji kodu, otrzymasz przypadek tego, co jest w zasadzie „kodem, którego nie lubisz”.
Czy nie powinieneś „kodu, którego nie lubisz”? Powiedziałbym „nie”. Naturalnie zaczniesz zdawać / nie zdawać w oparciu o aspekty inne niż kod: Czy podoba Ci się ta osoba? Czy zdecydowanie argumentują za swoją sprawą, czy po prostu robią, co im kazano? Czy przekazują Twój kod, gdy Cię sprawdzają?
Ponadto dodajesz niewymierny krok do procesu szacowania. Zadanie oceniam na podstawie tego, jak myślę, że należy je zaprogramować, ale na samym końcu muszę zmienić styl kodowania.
Jak długo to doda? Czy ten sam recenzent dokona kolejnej recenzji kodu i zgodzi się z pierwszym recenzentem, czy też znajdzie dodatkowe zmiany? Co się stanie, jeśli nie zgodzę się na zmianę i odłożę ją, szukając drugiej opinii lub argumentując sprawę?
Jeśli chcesz, aby zadania były wykonywane szybko, musisz je maksymalnie precyzyjnie określić. Dodanie niejasnej jakości bramy nie pomoże w wydajności.
Re: Nie można zapisać zasad !!
To nie jest takie trudne. Naprawdę masz na myśli „Nie mogę wyrazić tego, co mam na myśli przez„ dobry ”kod” . Kiedy to zauważysz, zobaczysz, że to oczywiście problem HR, jeśli zaczniesz mówić, że czyjaś praca nie jest do zera, ale nie możesz powiedzieć dlaczego.
Zapisz zasady, które możesz, i rozmawiaj przy piwie na temat tego, co czyni kod „dobrym”.
źródło