Jak obsługiwać TODO w żądaniu pull?

24

Kiedy przeglądam zmiany w żądaniu ściągnięcia, czasami natrafiam na komentarz z notatką „DO ZROBIENIA”, który może być tam z różnych powodów, w naszym przypadku głównie z powodu:

  • rozwiązanie zastosowane do rozwiązania problemu można ulepszyć, ale wymagałoby to znacznie więcej czasu. Autor wybrał szybsze rozwiązanie, ale skomentował, że lepsza opcja jest potencjalnie dostępna
  • istnieje tymczasowy kod do obejścia istniejącego błędu, który powinien zostać wkrótce naprawiony

Wiedząc, że TODOogólnie pozostanie w bazie kodu przez cały okres jej działania, jak mam na nie zareagować w żądaniu ściągnięcia? Jak mogę uprzejmie poprosić o uniknięcie tego, a jeśli jest to naprawdę uzasadnione, w jaki sposób mogę się upewnić, że autor PR podejmie dalsze kroki w przyszłości?

alecxe
źródło
Jeśli cofnięte czynności do wykonania stają się problemem dla Twojego zespołu, czy nie możesz przypisać kogoś (lub, jeśli sam nie masz takiego uprawnienia, poprosić szefa o wyznaczenie kogoś), aby poświęcił trochę czasu na przejrzenie całego kodu i wykonanie wszystkich zadań do wykonania?
nick012000,
Jednym ważnym czynnikiem jest to, czy dane TODO, o którym mowa, ma charakter, który mogą rozwiązać programiści inni niż autor. Innym czynnikiem jest pragmatyczna ocena ryzyka lub znaczenia tego TODO - czy to tylko płaczący wilk?
rwong
Posiadanie kilku TODO w bazie kodu nie stanowi żadnego problemu.
Oliver Watkins

Odpowiedzi:

26

Gdy powiesz, że „zazwyczaj pozostają w bazie kodu przez cały okres istnienia bazy kodów” w zespole / dziale / organizacji, weź pod uwagę następujące kwestie:

  • Zapisz go w DoD , że TODO, FIXMEczy należy unikać podobnych tagów.
  • Użyj narzędzia do analizy kodu statycznego, takiego jak SonarQube, aby automatycznie oznaczyć kompilację jako niestabilną.
  • Tymczasowo zezwól na nie tylko wtedy, gdy w narzędziu do śledzenia problemów znajduje się odpowiedni bilet. Wówczas kod może wyglądaćTODO [ID-123] Description ...

Jak wspomniano w moim komentarzu , ostatnie stwierdzenie prawdopodobnie ma sens tylko w środowisku, w którym bilety nie gniją (np. Jeśli przestrzegasz zasady zerowego błędu ).

Osobiście uważam, że czasamiTODO są rozsądne, ale nie należy ich nadmiernie wykorzystywać. Zaczerpnięte z „Clean Code: A Handbook of Agile Software Craftsmanship” Roberta C. Martina (s. 59):

TODOs są zadaniami, które zdaniem programisty powinny być wykonane, ale z jakiegoś powodu nie mogą w tej chwili wykonać. Może to być przypomnienie o usunięciu przestarzałej funkcji lub prośba o sprawdzenie problemu przez kogoś innego. Może to być prośba do kogoś o wymyślenie lepszego imienia lub przypomnienie o dokonaniu zmiany zależnej od planowanego wydarzenia. Cokolwiek TODOmoże być, to nie pretekst do opuszczenia zły kod w systemie.

beatngu13
źródło
2
Czy odpowiedni bilet nie pozostanie w zaległości na zawsze? Widziałem, że zarówno TODO, jak i bilety pozostają nierozwiązane na zawsze.
dzieciou
5
@dzieciou niekoniecznie, ale oczywiście istnieje ryzyko, że pozostanie tam również na zawsze. Jeśli jednak masz bilet, ryzyko to jest prawdopodobnie mniejsze w porównaniu z samym komentarzem do kodu. Myślę, że to zależy od zespołu / działu / organizacji, czy ma to sens.
13
Jeśli masz politykę zakazu wykonywania zadań, programiści po prostu zostawiają komentarz do zrobienia poza kodem i pozostawiają wątpliwy szybki i brudny kod w środku. Zły pomysł.
RibaldEddie,
8
Todos są formą komunikacji. Między programistami. Czasami przez długie odcinki czasu. Użycie słowa TODO w komentarzu do kodu jest niczym innym jak cukrem syntaktycznym dla szczególnego zainteresowania.
RibaldEddie,
2
Myślę, że kluczowa jest tutaj wzmianka o dodaniu go do narzędzia do śledzenia problemów. Jeśli to zrobisz, może się zdarzyć, że ludzie zaczną to naprawiać bez Twojej wiedzy. Widziałem, jak to się dzieje w organizacji; problemy zostały wykryte tylko dlatego, że ludzie lubili naprawiać błędy, kod refaktoryzujący itp. Czasami może być całkiem fajnie.
Per Lundberg,
5

TODO nie są złe. Jestem wielkim fanem nigdy nie sięgania głębiej niż na jedną warstwę i zawsze naprawiam 1 problem na bilet trackera.

Często używam TODO lub FIXME jako sposobu na przypomnienie sobie, że potrzebuję lub chcę wrócić, aby naprawić problem.

Na przykład mogę wywołać add (a, b) i dodać TODO, aby zmienić metodę dodawania. Nie chcę teraz pracować nad metodą dodawania, ale chcę do niej wrócić.

Więc w żądaniu ściągnięcia zobaczysz TODO lub FIXME. Używam FIXME na przykład, aby ostrzec innych twórców kodu o obszarach, za które są odpowiedzialni i że nie powinienem z tym zadzierać. Na przykład dodawanie FIXME nie może przyjmować liczb ujemnych.

Aby obejść problem, że nie są one widoczne lub ignorowane, używam skryptu, który wyświetla wszystkie wiersze TODO FIXME i DEGUG. I to jest dołączane do komunikatu zatwierdzenia.

Trudno zignorować komunikat zatwierdzenia linii 400, który jest wszystkimi TODO. Dlatego ludzie je naprawiają, dotykając już danego kodu lub tworząc nowe zgłoszenia i usuwając samodzielny kod problemu.

Szczerze mówiąc, upewniam się również, że każdy projekt ma wbudowany czas czyszczenia kodu. Tak, może być gotowy do uruchomienia 15, ale robił czyszczenie kodu od 15 do 30. (wydaje się to dziwne, ale jeśli kiedykolwiek zarządzałeś produktem, wiesz, że jeśli spróbujesz mieć zadania o niskiej widoczności przed uruchomieniem, nigdy nie będziesz mógł się do nich dostać. Coś innego zyska priorytet.)

Coteyr
źródło
1
Zgadzam się, że TODOsame w sobie nie są złe, ale używanie ich tak często uważam za hałas. Myślę również, że wiadomość zatwierdzenia 400 linii łatwo jest ignorowana, ponieważ ludzie często pomijają tak dużo tekstu. Co więcej, wiele nakładek Git / VCS (np. GitHub) obcina dowolny wiersz tematu dłuższy niż pewna liczba znaków.
beatngu13,
Tak, o to mi chodzi, na około 4-5 liniach ludzie chcą to posprzątać. Więc zaczynają robić TODO. 400 linii było przesadą.
coteyr
Byłbym zainteresowany tym, jak działa skrypt dodawania TODO do komunikatu zatwierdzenia.
Simon
Istnieje haczyk „commit-msg”, z którym można się powiązać.
coteyr
1

Zdarzy się, że są żądania ściągania, które nie są idealne. W tej chwili wykonuję pracę, którą można wykonać „wystarczająco dobrze” w dostępnym czasie, ale nie idealnie. Dlatego przesyłam żądanie ściągnięcia, umieszczam TODO z komentarzami we właściwych miejscach w kodzie, a jednocześnie dodaję kolejne żądanie zmiany, aby zmienić rzeczy z „wystarczająco dobrego” na „doskonałe”.

To nowe żądanie zmiany może następnie zostać uszeregowane według priorytetów i zostanie obsłużone, gdy będzie to element o najwyższym priorytecie. Jeśli zdecyduje się, że musi być teraz idealny , będzie to następna sprawa.

Teraz twoje pytanie: „Jak mogę uprzejmie poprosić, aby tego uniknąć, lub jeśli jest to naprawdę uzasadnione, w jaki sposób mogę się upewnić, że autor PR podąży za nim w przyszłości?” Z tym, co opisuję, wydaje się to raczej głupie. Jeśli mam wybór między późną wysyłką a wysyłką tego, co jest wystarczająco dobre, to nie twoja decyzja. Jeśli chcesz, możesz zapytać o to swojego menedżera produktu. I „upewnij się, że autor PR podąży za nim”? Jeśli masz zespół, powinieneś upewnić się, że jest on zalogowany w twoich systemach, i mam nadzieję, że twój zespół jest wystarczająco dobrze zorganizowany, aby zarejestrowane rzeczy się nie zgubiły, a ktoś w końcu to naprawi, gdy nie będzie elementów o wyższym priorytecie. Pamiętaj, że to wysiłek zespołu.

gnasher729
źródło
1

Jeśli twój projekt śledzi oczekujące elementy w kodzie źródłowym z TODOkomentarzami, musisz na to pozwolić.

Fakt, że obecność TODOkomentarza w żądaniu ściągnięcia jest błędna, powinieneś powiedzieć, że śledzenie oczekujących elementów w kodzie źródłowym jest złym pomysłem. W ten sposób rzeczy giną lub są ignorowane. Teraz, jeśli mówisz o żądaniu ściągnięcia do „działającego widelca”, sytuacja jest inna. „Roboczy widelec” jest właśnie tym - praca w toku. Ale taki rozwidlenie zwykle nie wymaga żądania ściągnięcia. Sugerowane tutaj „reguły domowe” dotyczą wniosków ściągania dla gałęzi master .

Zasada domowa nr 1 - wszystkie zatwierdzenia do masterowania powinny być gotowe do testowania, ponieważ master jest budowany codziennie po każdym zameldowaniu. Te zatwierdzenia powinny również obejmować wszelkie dodatkowe wymagane testy.

Jeśli TODOkomentarz istnieje, ponieważ kod nie został ukończony lub testy nie są zakończone lub kod nie jest w żaden sposób gotowy do testowania, kod ten należy do lokalnego zatwierdzenia, a nie żądania ściągania. Zadzwoń, kiedy będzie gotowy.

Zasada domowa nr 2 - Wszystkie informacje dotyczące otwartych problemów są przechowywane w narzędziu do śledzenia problemów. Wszystko. Notatki, bazgroły, przeczucia, cokolwiek.

Jeśli TODOkomentarz dotyczy otwartego problemu i nie jest faktyczną poprawką dla tego problemu, to ta informacja należy do narzędzia do śledzenia problemów. W ten sposób przed zamknięciem problemu wszystkie informacje można w razie potrzeby przejrzeć i zweryfikować, aby upewnić się, że problem został rzeczywiście rozwiązany.

Zasada domowa nr 3 - Wszystkie informacje dotyczące oczekujących zadań projektowych należą do kolejki priorytetowej (lub jakiejkolwiek innej nazwy twojego systemu).

Dla wyjaśnienia, oczekujące zadanie projektowe jest czymś, co należy wykonać w projekcie, który ma ustalony priorytet, niezależnie od tego, czy jest to defekt wykryty przed wygenerowaniem zgłoszenia problemu, czy też realizacja określonego wymagania projektowego lub jednego z niezbędne komponenty tego wymagania.

Jeśli w TODOkomentarzu jest powiedziane, że nowy kod wpłynie na oczekujące zadanie, lub wskazać coś innego w bazie kodów, które należy sprawdzić, co zostało odkryte podczas implementacji nowego kodu, to informacja ta należy do kolejki priorytetowej, albo istniejące zadanie lub jako nowe.

Zasada domowa nr 4 - sugestie należą do skrzynki pomysłów (lub cokolwiek innego).

Jeśli TODOkomentarz sugeruje zmianę w projekcie lub implementacji oprogramowania, to ta informacja należy do okna pomysłów projektu lub „vNext” lub „Uwagi do projektu”, lub cokolwiek innego, co masz do tego rodzaju rzeczy.

Reguła nr 5 - TODOkomentarze nie są dozwolone w kodzie źródłowym. KROPKA.

Jeśli będziesz trzymać się tej zasady, nie będziesz musiał się martwić, że ktokolwiek będzie cię śledził.

Mark Benningfield
źródło