Przepływ pracy przeglądania kodu, w którym autor żądania ściągania musi się scalić

Kilka zespołów w mojej firmie ćwiczy proces przeglądu kodu, którego nigdy wcześniej nie widziałem. Staram się zrozumieć, co za tym kryje się, z myślą, że warto zapewnić spójność całej firmy. (Przyczyniam się do wielu baz kodowych i byłem zaskoczony różnicami w przeszłości.)

1. Autor kodu wysyła żądanie ściągnięcia
2. Recenzent sprawdza kod
3. • Jeśli recenzent wyrazi zgodę, zostawia komentarz w stylu „Wygląda dobrze, nie krępuj się łączyć”
   • Jeśli recenzent ma wątpliwości, zostawia komentarz w stylu „Napraw drobne problemy X i Y, a następnie scal” (w przypadku poważnych zmian wróć do kroku 2)
4. Autor kodu dokonuje zmian, jeśli to konieczne, a następnie scala własne żądanie ściągnięcia

Mam następujące obawy:

• W przypadku zatwierdzenia w kroku 3 ten przepływ pracy tworzy pozornie niepotrzebną rundę do autora żądania ściągnięcia. Recenzent, który już patrzy na kod, może go natychmiast scalić.

• W przypadku żądania zmian w kroku 3 agencja do scalenia żądania ściągnięcia spoczywa teraz wyłącznie na autorze PR. Nikt oprócz autora nie przyjrzy się zmianom przed scaleniem.

Jakie są inne zalety lub wady tego przepływu pracy? Czy ten przepływ pracy jest powszechny w innych zespołach inżynieryjnych?

W pierwszym przypadku jest to zwykle uprzejmość. W większości organizacji fuzje rozpoczynają serię automatycznych testów, z którymi należy się szybko poradzić, jeśli się nie powiodą. Zwłaszcza jeśli wystąpiło znaczne opóźnienie między momentem złożenia żądania ściągnięcia a momentem jego przeglądu, uprzejmie jest zezwolić na scalenie go zgodnie z harmonogramem autora, aby mieli czas na poradzenie sobie z nieoczekiwanym wypadkiem. Najłatwiejszym sposobem na to jest samodzielne połączenie.

Czasami autor dowiaduje się później o przyczynach, dla których żądanie ściągnięcia nie powinno być jeszcze scalone. Być może PR innego dewelopera ma wyższy priorytet i może spowodować konflikty. Może pomyślała o nieosłoniętym przypadku użycia. Być może komentarz do recenzji wywołał burzę mózgów, która wymaga dalszych badań, zanim problem zostanie w pełni zaspokojony. Autor wie najwięcej o kodzie i sensowne jest podanie mu ostatniego słowa o tym, kiedy zostanie scalony.

Po drugie, to tylko kwestia zaufania. Jeśli nie możesz ufać ludziom, że naprawią drobne problemy bez dwukrotnego sprawdzenia, nie powinni dla ciebie pracować. Jeśli problem jest na tyle duży, że po poprawce będzie wymagał kolejnej recenzji, zaufaj recenzentom, aby poprosić o jedną.

To powiedziawszy, czasami łączę żądania ściągania innych autorów, ale zwykle są to albo bardzo proste zmiany, albo ze źródeł zewnętrznych, gdzie osobiście biorę odpowiedzialność za przechodzenie przez wszelkie awarie automatyzacji testów.

Posiadanie przez pierwszego autora scalenia własnego żądania ściągnięcia jest moim preferowanym procesem pracy w małych zespołach. Oprócz wspomnianych już zalet technicznych (na przykład w zakresie rozwiązywania konfliktów scalania), myślę, że wnosi wartość dodaną na poziomie kulturowym: buduje poczucie własności.

Podałem początkowego autora dla (rzadkiego) przypadku, gdy inny programista dodałby zatwierdzenia do otwartego żądania ściągania (ściągając gałąź funkcji w toku i wypychając z powrotem do niej). Nie zdarza się to często i wynikałoby z rozmowy osobiście lub nad Slackiem: te dodatkowe zatwierdzenia (przez kogoś innego) nie powinny tam lądować z zaskoczenia! W tym kontekście przez pierwszego autora rozumiem tego, który przesłał żądanie ściągnięcia.

W mojej organizacji jesteśmy nowi w żądaniach ściągania i twoje pytanie jest tym, nad czym się zastanawiałem.

Chciałbym dodać jedno spostrzeżenie: w niektórych narzędziach (korzystamy z TFS) może istnieć element pracy powiązany z żądaniem ściągnięcia.

W takim przypadku trudno jest śledzić, kiedy recenzent wykonał scalenie. W takim scenariuszu programista musi ponownie odwiedzić PR, otworzyć błąd lub zmienić żądanie i oznaczyć go jako „rozwiązany”. Jeśli oznaczymy go jako „rozwiązany” zbyt wcześnie, testerzy uważają, że poprawka jest już częścią bieżącej wersji.

TFS 2017 poprawił implementację żądania ściągnięcia. Teraz programista może poprosić o pobranie żądania, aby automatycznie scalało, jeśli wszystkie warunki są spełnione (brak konfliktu scalania, zatwierdzenie przez recenzentów i brak zepsutych wersji). YMMV.

W ten sposób autor ma szansę zmienić zdanie na temat swojej gałęzi, być może wymyślił coś, co powinno być zrobione w inny sposób, i odłożyć dodatkowe opłaty na ponowne sprawdzenie.