Czy przegląd kodu, który wykorzystuje tylko komentarze do kodu, jest dobrym pomysłem?

Warunki wstępne

• Zespół używa DVCS
• IDE obsługuje parsowanie komentarzy (np. TODO itp.)
• Narzędzia takie jak CodeCollaborator są drogie ze względu na budżet
• Narzędzia takie jak gerrit są zbyt skomplikowane do zainstalowania lub nie nadają się do użytku

Przepływ pracy

• Autor publikuje gdzieś w centralnej gałęzi funkcji repo
• Recenzent pobierz go i zacznij recenzję

• W przypadku niektórych recenzentów pytań / problemów utwórz komentarz ze specjalną etykietą, np. „REV”. Taka etykieta MUSI nie znajdować się w kodzie produkcyjnym - tylko na etapie recenzji:

  $somevar = 123;
  // REV Why do echo this here?
  echo $somevar;
  

• Kiedy recenzent kończy dodawanie komentarzy - po prostu popełnia głupią wiadomość „komentarze” i odpycha

• Autor wycofuje gałąź funkcji i odpowiada na komentarze w podobny sposób lub poprawia kod i wypycha go z powrotem
• Kiedy komentarze „REV” znikną, możemy pomyśleć, że przegląd został pomyślnie zakończony.
• Autor interaktywnie zmienia gałąź funkcji, zgniata ją, aby usunąć zatwierdzenia „komentarza”, a teraz jest gotowy do scalenia funkcji w celu opracowania lub wykonania dowolnej czynności, która zwykle mogłaby być przeprowadzona po udanej wewnętrznej ocenie

Obsługa IDE

Wiem, że niestandardowe tagi komentarzy są możliwe w eclipse i netbeans. Jasne, że powinno to również należeć do rodziny blablaStorm.

pytania

1. Czy uważasz, że ta metodologia jest wykonalna?
2. Czy znasz coś podobnego?
3. Co można w nim poprawić?

Pomysł jest bardzo fajny. W przeciwieństwie do typowych przepływów pracy, przeglądasz bezpośrednio w kodzie, więc technicznie nie potrzebujesz niczego oprócz edytora tekstu, aby korzystać z tego przepływu pracy. Wsparcie w IDE jest również miłe, szczególnie możliwość wyświetlania listy recenzji na dole.

Nadal istnieje kilka wad:

• Działa dobrze w przypadku bardzo małych zespołów, ale większe zespoły będą wymagać śledzenia tego, co zostało sprawdzone, kiedy, przez kogo iz jakim rezultatem. Chociaż faktycznie masz tego rodzaju śledzenie (kontrola wersji pozwala o tym wszystkim wiedzieć), niezwykle trudno jest go używać i wyszukiwać, i często wymaga ręcznego lub półautomatycznego przeszukiwania wersji.

• Nie wierzę, że recenzent ma wystarczającą ilość opinii od recenzenta, aby wiedzieć, jak faktycznie sprawdzono punkty .

  Wyobraź sobie następującą sytuację. Alice po raz pierwszy sprawdza kod Erica. Zauważa, że ​​Eric, młody programista, zastosował składnię, która nie jest najbardziej opisowa w faktycznie używanym języku programowania.

  Alice sugeruje alternatywną składnię i przesyła kod z powrotem do Erica. Przepisuje kod przy użyciu tej alternatywnej składni, która jego zdaniem jest zrozumiała poprawnie, i usuwa odpowiedni // BLAkomentarz.

  W następnym tygodniu otrzyma kod do drugiej recenzji. Czy byłaby w stanie pamiętać, że wypowiedziała tę uwagę podczas swojej pierwszej recenzji, aby zobaczyć, jak Eric ją rozwiązał?

  W bardziej formalnym procesie recenzji Alice natychmiast zauważyła, że ​​zrobiła uwagę, i zobaczyła różnicę w odpowiednim kodzie, aby zauważyć, że Eric źle zrozumiał składnię, o której mu powiedziała.

• Ludzie wciąż są ludźmi. Jestem prawie pewien, że niektóre z tych komentarzy znajdą się w kodzie produkcyjnym, a niektóre pozostaną śmieciami, będąc całkowicie nieaktualnymi .

  Oczywiście ten sam problem istnieje w przypadku każdego innego komentarza; na przykład w produkcji jest wiele komentarzy TODO (w tym najbardziej przydatny: „TODO: Skomentuj poniższy kod.”) i wiele komentarzy, które nie zostały zaktualizowane, gdy odpowiedni kod był.

  Na przykład oryginalny autor kodu lub recenzent może odejść, a nowy programista nie zrozumie, co mówi opinia, więc komentarz pozostanie na zawsze, oczekując, że ktoś będzie zbyt odważny, aby go usunąć lub faktycznie zrozumieć to mówi.

• To nie zastępuje recenzję twarzą w twarz (ale ten sam problem dotyczy również innych bardziej formalnego przeglądu, który nie odbywa się twarzą w twarz).

  Zwłaszcza jeśli oryginalna recenzja wymaga wyjaśnień, recenzent i recenzent rozpoczną rodzaj dyskusji . Nie tylko znajdziesz duże komentarze BLA, ale te dyskusje również zanieczyszczą dziennik kontroli wersji .

• Możesz również napotkać drobne problemy ze składnią (która istnieje również w przypadku komentarzy TODO). Na przykład, jeśli długi komentarz „// BLA” pojawi się w kilku wierszach, a ktoś zdecyduje się napisać go w ten sposób:

  // BLA This is a very long comment which is way beyond 80 characters, so it actually
  // fills more then one line. Since the next lines start with slashes, but not "BLA"
  // keyword, the IDE may not be able to show those lines, and display only the first one.
  

• I wreszcie jako drobna i bardzo osobista uwaga: nie wybieraj słowa „BLA” jako słowa kluczowego. Brzmi brzydko. ;)

Uzupełnię komentarze w kodzie o dokument towarzyszący. Podsumowałoby to ustalenia i trwałoby po usunięciu komentarzy. Zaletami tego byłyby:

• ścisłość. Jeśli osoba rutynowo nie sprawdza, czy przekazany wskaźnik nie ma wartości zerowej (lub innego typowego błędu początkującego w używanym języku), recenzent może zostawić dziesiątki komentarzy REV w tym celu, aw dokumencie może powiedzieć „ Znalazłem 37 miejsc, w których wskaźniki nie były sprawdzane jako pierwsze ”, nie wymieniając ich wszystkich
• miejsce pochwały. Komentarz REV this is a nice designwydaje się po prostu dziwny, ale moje recenzje kodu często zawierają zarówno zatwierdzenie, jak i poprawki
• interaktywność. Twój przykładowy komentarz brzmi „dlaczego to zrobiłeś?” i jest to bardzo typowy. Podejście tylko do komentowania nie obsługuje rozmowy. Osoba może zmienić kod i usunąć komentarz lub usunąć komentarz. Ale „dlaczego to zrobiłeś?” a „proszę to zmienić, to źle” to nie to samo.
• prowadzenie rejestru. Późniejszy recenzent może sprawdzić, czy kod rzeczywiście został zmieniony, czy też komentarze zostały właśnie usunięte. Mogą również sprawdzić, czy komentarze zostały „uwzględnione”, a programista nie popełnia już tych samych błędów w kolejnej recenzji.

Chciałbym również użyć elementu pracy do wykonania recenzji i udzielenia odpowiedzi na recenzję oraz powiązać z nią kontrole. Ułatwia to znajdowanie komentarzy w starym zestawie zmian i sprawdzenie, jak każdy z nich był obsługiwany w innym zestawie zmian.

Inni mówili o ograniczeniach tego podejścia. Wspomniałeś, że narzędzia takie jak gerrit były trudne do zainstalowania - proponuję spojrzeć na phabricator (http://www.phabricator.com). Jest to system sprawdzania kodu, z którego Facebook korzysta od lat, a ostatnio został otwarty. Nie jest trudny do zainstalowania, ma doskonałe przepływy pracy i rozwiązuje wszystkie problemy wymienione w innych komentarzach.