Zwinne praktyki: weryfikacja kodu - nieudana weryfikacja lub problem?

53

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.

Erdrik Ironrose
źródło
23
Jeśli więc nie można tego zrobić z powodu przeglądu kodu, jaki jest cel przeglądu? W tej chwili wydaje ci się, że musisz sprawdzić, czy coś działa , ale z pewnością jest to zadanie testu lub testera, a nie recenzji kodu.
VLAZ
21
Myślę, że w większości odpowiedzi brakuje ważnego punktu w twoim pytaniu: W przeciwnym razie zadanie pasuje do definicji „zrobione”. Czy wymienione przez Ciebie problemy są częścią tego, co Twój zespół uważa za „niewykonane” zadanie? Czy te kwestie nie są brane pod uwagę przy wykonywaniu zadania? Jeśli twoja definicja „gotowe” obejmuje „brak zapachu kodu”, zadanie po prostu nie jest wykonane.
Josh Part
1
@ErdrikIronrose, więc wygląda na to, że zmiana nie była zgodna ze standardem i prawdopodobnie nie była (tak łatwa) w utrzymaniu. Chociaż twój drugi komentarz wydaje się wskazywać, że zmiana nie była częścią żądania, w takim przypadku nie powinna być częścią recenzji kodu. Jeśli ktoś napisze poprawny i niestandardowy kod obok istniejącego brzydkiego hacka, to nie krępuj się, podnieś bilet na naprawę brzydkiego hacka i przekaż bieżącą recenzję kodu. Jeśli ktoś napisze kod, który jest poprawny, ale niezgodny ze standardem (jak wskazuje pytanie), nie wykonuj przeglądu kodu, dopóki nie zostanie wykonany poprawnie.
VLAZ
9
@ErdrikIronrose: Ach, więc sposób tworzenia kodu zapachowego nie został stworzony podczas pracy nad recenzowaną historią , ale już istniał? To ważne rozróżnienie - rozważ edycję pytania.
sleske
1
@vlaz powinieneś odpowiedzieć na swój komentarz
Ister

Odpowiedzi:

67

czy są jakieś nieodłączne problemy lub względy związane z podniesieniem biletu z tyłu recenzji, zamiast go zawieść?

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.

w recenzji odkrywamy funkcję

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?

Niepowodzenie przeglądu kodu, aby bilet nie zamykał się w tym sprincie, i odbieramy trochę morale, ponieważ nie możemy przekazać 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.

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.

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”).

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.

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.

W przeciwnym razie zadanie pasuje do definicji gotowego.

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.

Flater
źródło
26
„Poprawność i czystość kodu nie zależy od nastroju zespołu”. +1 za to samo, jednak jedynym zastrzeżeniem dla całej odpowiedzi byłoby dotrzymanie terminu. Jeśli niepowodzenie tej recenzji kodu oznacza, że ​​oczekiwana funkcja nie przejdzie do następnej wersji, musisz zrównoważyć czystość kodu z potrzebami klienta. Pamiętaj jednak, że niepoprawny kod, który dotrzymuje dziś terminu klienta, jutro jest problemem produkcyjnym.
Greg Burghardt
11
Świetna odpowiedź - stanowcza, ale nie niegrzeczna. Jednym ze stycznych punktów może być również: w jaki sposób mogliśmy zrobić recenzje kodu tak późno w sprincie, że nie można było przeprowadzić łatwego refaktora bez spowodowania niepowodzenia całego sprintu?
Daniel
@Daniel: Deweloper może być w inny sposób zaangażowany lub może to być problem z planowaniem. Czas między ukończeniem zadania a ukończeniem sprintu jest zwykle minimalny, ponieważ (w idealnym świecie) ludzie kończą swoje ostatnie zadanie sprintu w okolicach godziny zamknięcia sprintu. Recenzja / naprawa nie może trwać dłużej. lub alternatywnie, może deweloper po prostu nie jest obecny / dostępny przez resztę sprintu.
Flater
8
+1 Programiści mogą czuć się dobrze, gdy napisają dobry kod. Ominięcie kontroli jakości nie jest odpowiedzią na poprawę morale. Zresztą od czasu do czasu odrzucenie drobnych spraw prawdopodobnie nie pogorszy morale. Jeśli twoje morale cierpi z powodu regularnego nieprzeprowadzania kontroli jakości, odpowiedzią jest zrobienie czegoś z powodu ciągłej nieudanej kontroli jakości, a nie rezygnacja ze standardów.
jpmc26
1
@GregBurghardt: Ponieważ widziałem, że w wielu firmach nadużywany jest argument dotyczący terminu, zgadzam się na złą ocenę tylko wtedy, gdy zostanie utworzone i zaplanowane zadanie jego natychmiastowej refaktoryzacji na pierwszy sprint po wydaniu. Dodatkowy koszt czasu stanowi znaczącą barierę wejścia w obchodzenie jakości kodu.
Flater
38

Pod koniec 2-tygodniowego sprintu, a zadanie ma przegląd kodu [...] Łatwa praca z refaktorem.

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.

nvoigt
źródło
5
To właśnie odpowiedź przyszła mi do głowy. Jeśli każda historia jest wdrażana z własną gałęzią, nie odkładaj przeglądu i scalania gałęzi do końca sprintu. Zamiast tego utwórz żądanie ściągania, gdy tylko gałąź będzie uważana za gotową, i kontynuuj iterację w tej gałęzi, dopóki nie zostanie ona faktycznie wykonana, zatwierdzona i scalona. Jeśli proces ten nie zakończył się na końcu sprintu, historia nie jest skończona.
Daniel Pryden
20

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ć.

gnasher729
źródło
4
Myślę, że w tym przypadku zmiana była zbyt długą funkcją - jeśli wprowadziłeś funkcję 3000 linii, której wcześniej nie było (lub wcześniej była to funkcja 10 linii).
user3067860,
3
Zasadniczo ta odpowiedź jest dokładnie poprawna. W praktyce ..... Jeśli wszyscy programiści wierzą i ćwiczą dobre praktyki kodowania zrównoważone wysiłkiem, prawdopodobnie nie napotkasz tego problemu zbyt często, a wtedy ta odpowiedź będzie natychmiastowa. Jednak .... wydaje się, że zawsze jest jeden lub dwóch programistów, którzy robią wszystko szybko i brudnie, aby zaoszczędzić teraz 5 minut; podczas gdy ignorują godziny, dni lub miesiące, które dodają do pracy, która będzie później. W takich przypadkach odpowiedź ta jest tylko śliskim stokiem, od którego trzeba zacząć od nowa i przeprojektować cały system.
Dunk
+1, choć myślę, że powinieneś przeformułować ostatni akapit, aby wyróżnić się, że sprawdzanie kodu z problemami powinno być absolutnym wyjątkiem. To znaczy, że ktoś jest zablokowany, to za mało wymówki. Niepowodzenie pojedynczego sprintu również nie wygląda na wystarczającą wymówkę, a na pewno nie na wymówkę, którą można by powtarzać.
Frax
@ user3067860 Jeśli funkcja 10 linii została zamieniona w funkcję 3000 linii - wtedy wyraźnie się nie powiedzie. Jeśli zmieniłeś funkcję linii 3000 na 3010 - prawdopodobnie przejdzie. Ale co, jeśli zmieniłeś funkcję 100 linii (zwykle nieco za dużą) w funkcję 300 linii ( zdecydowanie za dużą)?
Martin Bonner wspiera Monikę
9

Niepowodzenie przeglądu kodu, aby bilet nie zamykał się w tym sprincie, i odbieramy trochę morale, ponieważ nie możemy przekazać biletu.

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.

xyious
źródło
„Drogi kliencie, nie możesz mieć swojej funkcji przez kolejne 2-3 tygodnie, ponieważ nasz kod działał, ale nie podobało nam się, jak to wyglądało”, ... nie idź do naszego konkurenta ... lub powiedz CEO !
RandomUs1r
6
@ Klienci RandomUs1r nie powinni mieć tego rodzaju informacji. Nie zostało to zrobione, ponieważ nie było na to czasu i to wszystko. Czy klienci decydują o sposobie pisania kodu? Jeśli zadzwonisz do elektryka, aby naprawić okablowanie w domu, czy mówisz „Po prostu zmień kable, ale nie zawracaj sobie głowy sprawdzaniem, czy są to właściwe kable”? A może mówisz swojemu lekarzowi: „Jestem chory - daj mi pigułki, ale najpierw mnie nie diagnozuj”? Przeglądy kodu powinny być nieodłączną częścią pracy, a nie czymś, co dyktuje klient.
VLAZ
1
@ RandomUs1r: „„ Drogi deweloperze, dlaczego ta funkcja nie została ukończona? ” - odpowiedź powinna brzmieć„ ponieważ nie mieliśmy wystarczająco dużo czasu, aby zbudować ją do akceptowalnego poziomu jakości ”, może po niej„ Możemy to dać tobie, jeśli chcesz pójść na kompromis w sprawie jakości ”
Bryan Oakley
1
@ RandomUs1r, więc zasadniczo chcesz poświęcić jakość kodu, co prawdopodobnie utrudni później wdrożenie funkcji. Poprawka 2-dniowa może teraz bardzo dobrze zaoszczędzić 4 tygodnie później. Zatem jest to: „Drogi kliencie, nie możesz mieć swojej funkcji przez kolejne 2-3 tygodnie, ponieważ teraz zajmuje to tak dużo czasu”. Czy to także koniec sprintu, czy też jest to ważny termin? Jeśli to ważny termin, mógłbym teraz zobaczyć połączenie, napisanie poprawki w ciągu najbliższych 2 dni i podniesienie PR zaraz po terminie.
xyious
5
Mówię tylko, że jeśli twoje standardy są różne pierwszego dnia i ostatniego dnia sprintu, to nie masz żadnego standardu, a twoja jakość nieuchronnie spadnie.
xyious
5

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.

  • Możesz po prostu wyczyścić kod i poinformować osobę o tym, co zrobiłeś. Zapewnia kilka możliwości mentoringu, ale powinny to być dość proste rzeczy, które można zrobić w ciągu kilku minut.
  • Możesz wykopać go z komentarzami, co jest nie tak. Jeśli obsługa błędów jest wykonywana źle lub programista powtarza te same błędy, można to uzasadnić.
  • Możesz stworzyć bilet i zaciągnąć dług techniczny. Bilet ma na celu upewnienie się, że zapłacisz go później. Może się zdarzyć, że jesteś w sytuacji kryzysowej i podczas przeglądu zmian widzisz większy problem niezwiązany bezpośrednio ze zmianą.

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).

Berin Loritsch
źródło
4

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ą:

  1. Jaki jest cel przeglądu kodu?
  2. W jaki sposób wyniki przeglądu kodu odnoszą się do definicji ukończenia dla elementu pracy?
  3. Jeśli przegląd kodu ma zastosowanie jako test bramkowania, jakie problemy są uważane za „blokery”?

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”.

Jay S.
źródło
1

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:

  1. 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.

  2. 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ę.

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.

W tym przypadku, że myślisz o stworzeniu osobnej historii do refaktoryzacji, jest to znak ostrzegawczy z kilku stron:

  1. Nie myślisz o refaktoryzacji jako o części kodowania, ale o osobnej operacji, która z kolei może spowodować upadek pod presją.

  2. 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.

  3. 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).

Curt J. Sampson
źródło
1

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):

Odkrywamy lepsze sposoby tworzenia oprogramowania, robiąc to i pomagając innym. Dzięki tej pracy doceniliśmy:

  • Osoby i interakcje dotyczące procesów i narzędzi
  • Działające oprogramowanie w obszernej dokumentacji
  • Współpraca z klientami w zakresie negocjacji umów
  • Reagowanie na zmianę po wykonaniu planu

Oznacza to, że chociaż w przedmiotach po prawej stronie znajduje się wartość, bardziej cenimy przedmioty po lewej stronie.

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.

Ant P.
źródło
Jestem za współpracą. Ale jakiego terminu użyłbyś, gdyby nie „nie”? Nawet dyskutując, jako grupa, jedna osoba powiedziałaby „to nie wystarczy, wymaga refaktoryzacji”, co oznacza po prostu, że nie przeszła kontroli jakości, prawda?
Erdrik Ironrose,
1
@ErdrikIronrose Nigdy nie użyłem - ani nie potrzebowałem użyć - terminologii „nieudana” weryfikacja kodu. Ktoś przegląda kod, następuje dyskusja na temat potencjalnych punktów poprawy, a następnie decyzja, czy rozwiązać te problemy. Nie wiąże się to z „przejściem” lub „porażką”, tylko komunikacja i postęp. Nie jestem pewien, dlaczego istnieje potrzeba pieczątki.
Ant P
0

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.

railsdog
źródło
0

Moim zdaniem istnieją dwa sposoby spojrzenia na ten problem:

  1. Sposób akademicki
  2. Prawdziwy sposób na świecie

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.

RandomUs1r
źródło
2
nikt w prawdziwym świecie nie podąża zwinnie do T - nie będzie już „zwinny”, jeśli będziemy mieć zbyt surowe zasady, prawda?
Paŭlo Ebermann
@ PaŭloEbermann Miałem zabawną rozmowę z firmą, z którą przeprowadziłem kiedyś wywiad. Twierdzili, że ich proces nie był zwinny, ponieważ nie był to podręcznikowy przykład zwinności. Mimo że wszystko, co robili, było w duchu zwinności. Zwróciłem na to uwagę, ale spotkałem się tylko z (zasadniczo) „Nie, nie stosujemy ustalonej zwinnej procedury do litery, nawet jeśli mocno zapożyczamy koncepcje. Dlatego nie jesteśmy zwinni”. To było dość dziwne.
VLAZ
Jak zauważyli inni recenzenci, w tym przypadku potencjalnie można wyciągnąć naukę z faktu, że kod nie przeszedł pomyślnie recenzji. Wydaje mi się, że ludzie z tego projektu naprawdę nie rozumieją dobrze, że a) musisz zostawić czas na sprawdzenie i poprawki dla każdej historii, oraz b) refaktoryzacja konieczna do pozostawienia czystego kodu jest istotną częścią fabuła. W takim przypadku najlepiej jest zawieść historię, aby wyjaśnić, że te rzeczy naprawdę nie są opcjonalne.
Curt J. Sampson,
@Cur Rozumiem, że mój może być niepopularny z punktu widzenia deweloperów (jestem deweloperem zbyt btw), ale biznes naprawdę powinien być na pierwszym miejscu, podpisują wypłaty i zasługuje na pewien szacunek. Jeśli chodzi o pozostawianie czasu, ponownie rzucę wyzwanie twojemu zrozumieniu prawdziwego świata, i musisz zdać sobie sprawę, że nie zawsze jest to możliwe, a wiele sprintów biegnie naprzód, ponieważ deweloperzy potrzebują rzeczy do zrobienia również pod koniec sprintu. Nie jest tak, ponieważ kod jest SOLIDNY, dział może podnosić stopy o 1/10 dni co 2 tygodnie i nic nie robić, co może być świetne w krótkim okresie, ale nie jest opłacalne długo.
RandomUs1r
@ RandomUs1r Pracuję również w prawdziwym świecie, cały czas wybieram skróty i zawsze stawiam na pierwszym miejscu, więc nie sądzę, że tutaj brakuje mi zrozumienia. Ale opis OP nie brzmiał: „zwykle zawsze mamy rację, a to był zwykły drobny beknięcie”, inaczej nie zadałby pytania. Jak wyjaśniłem w mojej odpowiedzi , wygląda to na problem z procesem, a naprawiasz go, ćwicząc prawidłowe wykonywanie procesu przed relaksacją.
Curt J. Sampson
-2

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.

  1. „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).

  2. „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”.

Ewan
źródło
6
Nie, nie rozumiesz, że „posiadanie idealnego i uniwersalnego standardu bez dwuznaczności” nie jest realistycznym warunkiem wykonywania recenzji kodu. Zawsze będą pojawiać się nowe rodzaje problemów, których jeszcze nie uwzględniliście, dlatego musisz być w stanie podjąć decyzję na nieznanym terytorium. Oczywiście powinieneś następnie udokumentować tę decyzję, aby nie było to już niezbadane terytorium, ale twoja odpowiedź opiera się na założeniu, że możesz w jakiś sposób zagwarantować brak niezbadanego terytorium, jeśli tylko opracujesz idealne zasady przed dokonaniem przeglądu. Stawiasz wózek przed koniem.
Flater
5
Absolutnie takie jak „funkcje muszą być mniejsze niż x linii” również nie są odpowiedzią .
Blrfl,
2
Uzgodniony z Blrfl. Funkcje (ogólnie) nie powinny przekraczać 20 linii. Ale uczynienie go absolutną regułą jest błędem. Szczególne okoliczności zawsze przeważają nad ogólnymi zasadami: jeśli masz dobry powód, aby uczynić swoją funkcję dłuższą niż 20 linii, zrób to.
Matt Messersmith,
1
Nie powinieneś potrzebować reguł dla kodu napisanego zgodnie ze specyfikacją prawną ... Możesz po prostu mieć wytyczne plus fakt, że wszyscy jesteście prawdopodobnie dorosłymi, którzy próbują osiągnąć ten sam cel końcowy (działający, czytelny, łatwy do utrzymania kod). Fakt, że wszyscy członkowie zespołu naprawdę zainwestowali w zespół i byli gotowi do wspólnej pracy, ma kluczowe znaczenie dla Scruma, więc jeśli go nie masz, być może Scrum nie jest dla twojego zespołu.
user3067860,
2
@Ewan Sure. Chodziło mi tylko o to, że OP ma wytyczne dotyczące funkcji , a nie regułę. Niezależnie od tego, gdzie próg jest ustawiony, zawiera porady, które pomagają ludziom znaleźć trudny do utrzymania kod, ale nigdy nie jest to absolutna zasada. Jeśli (jak twierdzi OP) jest on rzeczywiście doskonale czytelny, a recenzenci zgadzają się, że jest doskonale czytelny i nie ma problemów z jego odpowiednim testowaniem, to funkcja z definicji ma odpowiedni rozmiar. Recenzja może otrzymać notatkę w jednym wierszu z informacją: „Tak, jest dłuższa niż zalecana, ale zgadzamy się, że jest OK” i praca wykonana. Refaktoryzacja po tym punkcie jest złoceniem.
Graham