Jak tworzyć lepsze recenzje kodu, gdy żądania ściągania są duże?

Oświadczenie: Istnieje kilka podobnych pytań, ale nie znalazłem żadnego, który dotykałby konkretnie problemów, które napotykasz podczas przeglądania dużego żądania ściągnięcia.

Problem

Wydaje mi się, że moje recenzje kodu mogą być wykonane lepiej. Mówię szczególnie o dużych recenzjach kodu z wieloma zmianami w ponad 20 plikach.

Łatwo jest złapać oczywiste problemy z lokalnym kodem. Zrozumienie, czy kod spełnia kryteria biznesowe, to inna historia.

Mam problemy z myśleniem autora kodu. Jest to dość trudne, gdy zmiany są liczne i obejmują wiele plików. Staram się skupić na grupach plików związanych z konkretną zmianą. Następnie przejrzyj grupy jeden po drugim. Niestety narzędzie, którego używam (Atlassian Bitbucket), nie jest zbyt pomocne. Za każdym razem, gdy odwiedzam plik, jest on oznaczany jako widoczny, chociaż często okazuje się, że nie jest powiązany z aktualnie badanym fragmentem zmian. Nie wspominając o tym, że niektóre pliki powinny być odwiedzane wiele razy, a ich zmiany sprawdzane krok po kroku. Również powrót do odpowiednich plików, gdy podążasz złą ścieżką, nie jest łatwe.

Możliwe rozwiązania i dlaczego nie działają dla mnie

Przejrzenie żądania ściągnięcia przez commity często rozwiązuje problemy z rozmiarem, ale nie podoba mi się to, ponieważ często będę patrzeć na nieaktualne zmiany.

Oczywiście, tworzenie mniejszych żądań ściągania wydaje się lekarstwem, ale tak właśnie jest, czasami otrzymujesz duże żądanie ściągnięcia i trzeba je sprawdzić.

Możesz również zignorować logiczny aspekt kodu jako całości, ale wydaje się to dość ryzykowne, szczególnie gdy kod pochodzi od niedoświadczonego programisty.

Pomocne może być użycie lepszego narzędzia, ale go nie znalazłem.

pytania

• Czy masz podobne problemy z recenzjami kodu? Jak sobie z nimi radzisz?
• Może masz lepsze narzędzia?

Mieliśmy te problemy i zadawanie poniższego pytania działało dla nas dobrze:

Czy PR robi jedno, co można połączyć i które można niezależnie przetestować?

Staramy się przełamać PR przez pojedynczą odpowiedzialność (SR). Po początkowym odparciu ludzie byli zaskoczeni, że nawet coś małego, choć singiel może być duży.

SR ułatwia przegląd, a także upowszechnia wiedzę na temat oczekiwanego wdrożenia.

Pozwala to również na przyrostowe reaktory, ponieważ dodaje się więcej, a czas realizacji PR jest drastycznie skrócony!

Sugeruję podzielenie ich przez SR, jeśli to możliwe, i zobacz, czy to działa dla ciebie. Potrzeba trochę praktyki, aby zrobić to w ten sposób.

Czasami nie da się uniknąć dużych próśb ściągania - ale możesz rozeznać, kto ponosi za to odpowiedzialność.

Traktuję żądania ściągania jako przekonujące argumenty. Autor próbuje mnie przekonać, że kod powinien wyglądać i działać w ten sposób.

Jak w przypadku każdego argumentu, powinien mieć jeden jasny pomysł. To albo:

• refaktor,
• optymalizacja,
• lub nowa funkcjonalność.

Jeśli nie są jasne, istnieje duża szansa, że ​​sami tego nie zrozumieją. Otwórz dialog i pomóż im rozbić ich argumenty na pod-argumenty. W razie potrzeby jest całkowicie w porządku - nawet dla nich korzystne jest odtworzenie tych zmian i zaoferowanie bardziej zrozumiałych i bezpośrednich żądań ściągnięcia.

Nadal będą pojawiać się duże żądania ściągania, ale z jasnym argumentem znacznie łatwiej jest zobaczyć, co nie pasuje.

Jeśli chodzi o oprzyrządowanie, zależy to od organizacji i procesu. BitBucket jest przyzwoitym narzędziem, niezależnie od tego, czy jest ono lepsze, czy nie, zależy od wszystkiego, od budżetu, sprzętu, wymagań, aż po istniejące procesy, reguły biznesowe i różne dostosowania oprogramowania, które już wprowadziłeś, aby pomieścić BitBucket. Zacznę od przejrzenia dokumentacji, aby sprawdzić, czy zachowanie można skonfigurować, być może rzucając się do społeczności wtyczek (lub przyłączając się, tworząc wtyczkę, aby to zrobić).

Nie sprawdzaj kompletnego żądania ściągania, ale każde zatwierdzenie. I tak uzyskasz lepsze zrozumienie żądania ściągnięcia, robiąc to w ten sposób .¹

Jeśli zatwierdzenia są małe, wykonanie takiej recenzji nie powinno stanowić problemu. Śledzisz zmiany jeden po drugim przez zatwierdzenia i ostatecznie otrzymujesz pełny obraz. Istnieją pewne wady, takie jak to, że czasami przeglądasz zmiany, które później cofniesz kilka zmian, ale nie powinno to być świetne.

Jeśli zatwierdzenia są duże, powinieneś omówić to z osobą, która je wykonała. Wyjaśnij mu, dlaczego duże zobowiązania są problematyczne. Słuchaj argumentów drugiej osoby na temat tego, dlaczego rzadko popełnia zmiany: możesz dowiedzieć się na przykład, że unika się dokonywania zmian, ponieważ przechwytywanie zatwierdzeń zajmuje zbyt dużo czasu, w którym to przypadku problem należy rozwiązać w pierwszej kolejności.

Przejrzenie żądania ściągnięcia przez commity często rozwiązuje problemy z rozmiarem, ale nie podoba mi się to, ponieważ często będę patrzeć na nieaktualne zmiany.

Robisz, ale jest to drobny problem i powinieneś tracić znacznie mniej czasu na przeglądanie kodu, który zostanie cofnięty o kilka zatwierdzeń później, niż czas, który marnujesz, próbując dowiedzieć się, dlaczego kod został zmieniony podczas przeglądania pojedynczego pliku.

„Często” jest niejasne, ale jeśli spędzasz zbyt dużo czasu na sprawdzaniu kodu, który nie znalazł się w końcowej wersji żądania ściągnięcia, możesz użyć dwóch technik:

• Szybko przejrzyj wszystkie zatwierdzenia raz, po prostu czytając komunikaty zatwierdzeń. W ten sposób, podczas studiowania konkretnego zatwierdzenia, możesz pamiętać, że komunikat zatwierdzenia gdzieś później powiedział, że zmiana, na którą patrzysz, została cofnięta.

• Mieć widok obok siebie najnowszej wersji pliku (chociaż w wielu przypadkach, w przypadku dużych zmian, (1) pliki mogą być radykalnie różne i (2) nazwy plików mogą ulec zmianie lub duże bloki kodu mogą zostać przeniesione w inne miejsce, co utrudnia znalezienie pasującego pliku).

• Poproś podmioty zatwierdzające, aby zmiażdżyły zatwierdzenia, gdy ma to sens, lub zastosuj określone konwencje komunikatów zatwierdzania, w których jedno zatwierdzenie wycofuje część innego, i dokonaj przeglądu, biorąc pod uwagę kilka kolejnych zatwierdzeń.

^{¹ Na przykład wyobraź sobie, że patrzysz na plik, w którym zmieniono nazwę jakiejś zmiennej. Co ci to mówi? Jak przejrzeć tę zmianę? Gdybyś jednak sprawdzał zatwierdzenie przez zatwierdzenie, zobaczyłbyś, że pojedynczy zatwierdzenie zmienił nazwę tej samej zmiennej w dwudziestu plikach, a komentarz wskazuje, że nazwa została zmieniona w celu użycia właściwego terminu biznesowego. Ta zmiana ma sens i można ją przejrzeć.}

Sprawdź, co próbujesz osiągnąć, przeglądając żądania ściągania i sprawdź, czy istnieje alternatywa.

Na przykład możesz chcieć

• Upewnij się, że przestrzegane są standardy
• Sprawdź, czy funkcjonalność jest poprawna
• Upewnij się, że więcej niż jedna osoba rozumie kod
• Trenuj juniorów

itd itd.

Niektóre z nich mogą być lepiej obsługiwane przez inne rzeczy, a nawet samo zrozumienie przyczyn pozwala ograniczyć zakres kontroli.

Na przykład, jeśli sprawdzam każdą linię, aby zasugerować i omówić zmiany dotyczące treningu, mogę pominąć to w przypadku dużych PR przeprowadzonych przez seniorów

Jeśli muszę zrozumieć kod, może sparuj programowanie dużych funkcji i ogranicz weryfikację kodu do kontroli standardów.

Nie musisz sprawdzać wszystkich rzeczy na każdym PR, dopóki masz warstwowe podejście do kontroli jakości