notacja commitów - jak dwa proste pytania zrewolucjonizują twoją pracę

[Zobacz słownik używanych pojęć]

Największa zmianą w moim stylu pracy było przyjęcie notacji commitów opartej na dwóch pytaniach:

  1. jakiego typu jest ta zmiana?
  2. jak duże ryzyko wnosi?

To sprawiło że praca stała się prostsza, szybsza, bezpieczniejsza i dająca większą satysfakcję.

1. jakiego typu jest ta zmiana?

W notacji możemy zawrzeć masę informacji ale najważniejszą jest typ zmiany. Jakiego typu może być zmiana?

Table 1: Typy zmian zaczerpnięte z książki "Living Documentation: Continuous Knowledge Sharing by Design" by Cyrille Martraire
typ zmiany opis zmiany
feat A new feature
fix A bug fix
docs Documentation-only changes
style Changes that do not affect the meaning of the code, such as changes to whitespace, formatting, missing semicolons, and so on
refactor A code change that neither fixes a bug nor adds a feature
perf A code change that improves performance
test Changes that add missing tests
chore Changes to the build process or auxiliary tools and libraries, such as documentation generation
remove Changes that involve removing something from the codebase (ten typ dodałem sam)

Jeśli zaczniemy określać typ każdej zmiany, commita to stanie się kilka rzeczy:

  1. przestaniemy mieszać ze sobą różne typy zmian, zaczniemy tworzyć commity jednego, góra dwóch typów
  2. zmniejszy się rozmiar commita co ułatwia ich przegląd i integrację
  3. zwiększy się nasze skupienie, skupiamy się tylko na jednym typie zmiany, na jednym celu który chcemy osiągnąć. Nie rozpłyniemy się myślami i czynnościami w innym kierunku
  4. zwiększy się nasza satysfakcja z pracy. Praca zostanie podzielona na kilka mniejszych checkpointów, a po osiągnięciu każdego z nich będziemy mogli głośno obtrąbić nasz sukces!
  5. zwiększy się bezpieczeństwo pracy i obniżymy ryzyko wystąpienia błędu
  6. wymusi to chęć częstszej integracji - Continuous Integration (CI)
  7. zachęci to nas do robienia drobnych poprawek - refactorings!
  8. stworzymy szczegółową dokumentacje dotyczącą zmian w projekcie
  9. otworzy się furtka do takich technik jak 'read by refactoring'

Jak widać warto, warto. Ale równie dobrze mógłbym wypisać zalety codziennych ćwiczeń fizycznych. Wszystko fajnie ale jak to wprowadzić, żeby ludzie chcieli z tego korzystać?

Na notacje bezpośredni wpływ ma sposób integracji kodu. Notacja i integracja są ze sobą ściśle powiązane i wzajemnie na siebie oddziałują. Ważniejsza jest notacja, bo jest pierwsza: wpierw opisujemy zmiany, następnie integrujemy kod. Więc jeśli wyjdziemy od notacji to sposób integracji kodu zostanie naturalnie wymuszony.

Natomiast działa to też w drugą stronę, jeśli jeszcze nie opisujesz typu zmian to zapewne korzystasz ze sposobu integracji kodu zwanego 'feature branch' i to przez niego masz trochę wymuszoną notacje: 1 commit = 1 ticket. Przyjrzyjmy się więc sposobom integracji kodu.

2. feature branch

Tworzę nowy branch z gałęzi, która jest stabilna, np. master i na nim wprowadzam zmiany. Dopiero kiedy skończę, integruję go z masterem. Może zawierać kilka commitów albo jeden. Często w momencie integracji te kilka commitów jest łączonych w jeden przez opcję 'squash commits in a merge request'.

2.1. zalety

  • master nie zawiera niedokończonych ficzerów, kodu 'in-progress'
  • czysta i prosta historia commitów, nowych ficzerów
  • pozwala łatwo wycofać dany ficzer - 'revert commit'
  • wymusza tworzenie małej liczby commitów

2.2. wady

  • im dłużej trwają pracę na takim wyizolowanym branchu tym więcej dodatkowej pracy musimy poświęcić na integracje z masterem
    • aby zmniejszyć problem często w trakcie prac aktualizujemy nasz branch z masterem
  • nie widzimy codziennych stopniowych zmian, tylko duże zmiany przy integracji
  • kiedy mam kilka branchy których zmiany dotyczą tych samych obszarów, to ich późniejsza integracja stwarza potworne problemy
  • duże skomplikowane, problematyczne MR

Ale najpoważniejszą wadą są problemy z refaktoringiem, pracami porządkowymi, przygotowującymi do właściwej zmiany. Co sprawia, że często ten rodzaj prac jest porzucany bo najzwyczajniej tutaj nie pasuję, tworzy wiele konfliktów, dodaje za dużo szumu, sprawia problemy.

2.2.1. problemy z refaktoringiem

Sądzę, że nie da się całkowicie uniknąć refaktoringu. W miarę jak problemy się nawarstwiają, będziemy zmuszeni je rozwiązać.

Problemy to narastające trudności w pracy, spowolnienie jej tempa, liczne błędy wynikłe z braku przejrzystości i czytelności kodu, coraz więcej czasu potrzeba na 'utrzymanie', naprawy rzeczy bieżących, niskie morale i duża niechęć do pracy.

Czasem mogą pojawić się pomysły na wygospodarowania większej ilości czasu zarezerwowanego tylko na refaktoring a raczej bardziej remodeling i posprzątaniu wielkiego bałaganu. To tak jak ze zdrowiem, albo dbamy o nie codziennie, albo raz na jakiś czas jesteśmy zmuszeni do zajęcia się nim i podreperowania. Nie ma tutaj ucieczki.

Jakie mamy możliwości, strategie kiedy chcemy uwzględnić taki rodzaj prac?

  1. robimy refaktoring raz na jakiś czas

    Jako całkowicie odrębne zadania a nawet cały sprint z odrębnymi zadaniami. Próbujemy wygospodarować osobny wyznaczony czas na refaktoring. Ale rzadko spotyka to się ze zrozumieniem ze strony managementu. Sytuacja pogarsza się, gdy musimy wprowadzić to w korporacyjne metody agile i oszacować czas realizacji.

    Refaktoring to małe inkrementalne zmiany a wygospodarowania większej ilości czasu to raczej grubsze próby przemodelowania całej aplikacji.

  2. robimy refaktoring codziennie i…
    1. nie określamy typu każdej zmiany (commita) i robimy jeden duży MR

      Tworzy to gigantyczne MR gdzie wymieszane są różnego typy zmiany. Koszmarnie się to przegląda, brak precyzyjnej informacji czego dotyczą zmiany. Zmiany niskiego ryzyka wymieszane są ze zmianami wysokiego ryzyka, jest ich po prostu za dużo. Takie MR powinno zostać bezwzględnie odrzucone.

    2. określamy typ każdej zmiany (commita) i robimy jeden duży MR

      Wydzielenie każdej zmiany to bardzo duży postęp. Jednak z uwagii, że wszystko jest w jednym MR to rzadko kiedy każdy commit jest odpowiednio wyizolowany, atomiczny, żeby w razie wątpliwości zintegrować tylko te wybrane. Powoduję do gwałtowny wzrost i tak już dużego ficzer brancha. Ale możemy osobno przejrzeć wydzielone commity.

    3. określamy typ każdej zmiany (commita) i każdy typ puszczamy jako osobny, mały MR albo w paczkach

      Myślę, że to najrozsądniejsze podejście. Oczywiście będziemy musieli zwiększyć częstotliwość integracji, ale zgodnie z podejściem 'feature branch' cały osobny ficzer puścimy tradycyjnie na koniec jako jeden commit, a codziennie będziemy integrowali tylko drobne zmiany. Sądzę, że jest to okres przejściowy, ale nadal potrzebny, aby zmiana podejścia i przyzwyczajeń przebiegła w miarę przyjemnie i płynnie.

      Zwiększy to czas potrzebny na 'code review', ale możemy umówić się w zespole, że pewne zmiany, które niosą niskie ryzyko, jak zmiany stylu, formatowania czy też automatyczne przekształcenia IDE, mogą być z automatu integrowane. Czyli pewna praca przygotowawcza niskiego ryzyka jest integrowana od razu, albo codziennie wystawiana jako MR, a zespół rano ma przeznaczyć np. 30 min na integrację, a my pracujemy dalej nad danym ficzerem. Więc siłą rzeczy musimy zwiększyć częstotliwość integracji.

      Integrujemy drobne zmiany, a cały czas staramy się utrzymać 'feature branch', który składa się z głównych, zaplanowanych zmian w tickecie. Wymuszamy częstszą integrację, ale tylko z drobnymi zmianami. Owy ficzer cały czas jest oddzielnym, całościowym branchem. Krok dalej to rozbicie go na mniejsze części i zwiększenie tempa integracji, czyli 'continuous integration (CI)'.

3. continuous integration (CI)

Przeciwieństwem 'feature branch' jest podejście Continuous Integration (CI). W którym codziennie integrujemy małe zmiany do mastera. Jeśli dodajemy jakiegoś ficzera to kod dzielimy na mniejsze porcję i dopóki całość nie zostanie skończona to ficzer jest wyłączony poprzez 'feature toggle'.

Małe, drobne zmiany to kwintesencja refaktoringu. Więc 'CI' tworzy idealne warunki dla tego typu działań.

4. porównanie feature branch vs. continuous integration (CI)

  feature branch continuous integration
commit: wielkość duży mały
commit: typ brak rozróżnienia na typy, albo 1 typ ficzer wiele typów
commit: ilość mała, zazwyczaj 1 ticket = 1 commit bardzo duża
integracja: kiedy? rzadko, bardzo problematyczna, czasochłonna codzienna, prosta, szybka
integracja: jak? całościowa, dopiero kiedy ficzer jest gotowy fragmentaryczna, po kawałku, codziennie
refaktoryzacja bardzo problematyczna, trudna, zniechęcająca bardzo prosta, łatwa, zachęcająca do zmian
ryzyko błędu wysokie, trudne do wykrycia i naprawienia niskie, łatwe do wykrycia i naprawienia
code review trudne, czasochłonne, skomplikowane duża ilość commitów ale są proste, małe, jednorodne

Myślę, że rysunek poniżej dobrze oddaje style pracy który porównujemy.

importance-of-small-steps.jpeg

Albo robimy duże zmiany i rzadko je integrujemy albo małe zmiany i często je integrujemy.

5. jak zacząć oznaczać typ commita?

Zacząłbym od najprostszych typów i wprowadzał stopniowo. Zapewne każdy oznacza 'feature' i 'fix', dodałbym do tego 'style' i 'refactor'.

5.1. style

Powszechnie wszyscy wiemy, aby nie mieszać zmian logiki oraz formatowania. Wybitnie zmniejsza to czytelność, czasem wręcz uniemożliwiając zrozumienie istoty zmian i jest podstawą do całkowitego odrzucania MR.

Powszechne narzędzia z których korzystamy (Git, Github, PHPStorm) nie rozróżniają typu zmian tylko wskazują linie w której zaszła dana zmiana nie wnikając w strukturę kodu.

Na przykład poniżej mamy prostą funkcję która z uwagii na brak wcięcie jest trochę 'nieczytelna', 'nieprzyjemna'.

function greet($name) {
echo "Hello $name";
}  

Wystarczy dodać wcięcie.

function greet($name) {
    echo "Hello $name";
}  

Jednak w diffie podświetlona będzie cała linia. Trzeba się przyjrzeć, zastanowić, może coś w 'Hello' się zmieniło, albo nazwa zmiennej. Musimy przyjrzeć się całej podświetlonej linii.

function greet($name) {
-echo "Hello $name";
+    echo "Hello $name";
 }

5.2. refactor

Opieramy się tutaj tylko na 'automatycznych przekształceniach' oferowanym przez IDE (np. PHPStorm) chodzi głównie 'refactorings': rename, inline, extract. Nie robimy nic manualnie, bo możemy się pomylić i wprowadzić błąd. Zaczęcie stosowania tego typu umożliwia zastosowania techniki 'read by refactoring', która jest niezwykle pomocna w pracy z kodem trudnym jak np. 'legacy code'.

6. read by refactoring

Dosłownie 'czytanie przez refaktoryzację'. Naszym celem jest zrozumienie kodu i aby to osiągnąć czytamy go jednocześnie poddając 'automatycznym przekształceniom' oferowanym przez IDE (np. PHPStorm) chodzi głównie 'refactorings': rename, inline, extract.

Czytając kod wprowadzamy do niego małe inkrementalne zmiany, które polepszają jego czytelność i strukturę. Co pozwala nam na szybsze, dokładniejsze i głębsze zrozumienie. Nie są to zmiany dotyczące zmiany zachowania kodu, lecz tylko te dotyczące nazewnictwa i struktury.

Myślę, że tutaj ten sposób można rozbić na dwa podejścia:

6.1. uwzględniamy wszystkie wykonane zmiany

Czyli każdą zmianę którą wykonamy robimy z zamiarem, że dołączymy ją do mastera. Aby mieć pewność, że zmiany nie niosą żadnego ryzyka korzystamy tylko z 'automatycznych przekształceń'. Nie możemy sami 'manualnie' zmieniać kodu bo niesie to ryzyko błędu.

6.2. nie uwzględniamy wszystkich zmian, tylko niektóre

Tutaj wykraczam poza bezpieczne i automatyczne przekształcenia IDE. Więc wprowadzamy ryzyko, które może doprowadzić do błędu. Moim celem jest tutaj uporządkowanie kodu tak jak mi się podoba, co ma pozwolić na jego szybsze i głębsze zrozumienie. Dopiero na podstawie uzyskanej wiedzy, insightu tworzę nowy branch i czasem przenoszę niektóre commity, które uważam, że są bezpieczne i wartościowe.

Poniżej przykład takiej zabawy:

6.2.1. przykład zastosowania dla trudnego kodu

Kod był dla mnie trudny, ponieważ był napisany w języku, w którym mam mało doświadczenia - elisp. Tutaj link do repozytorium: org-colview.

Pierwszy commit dotyczył zmiany komentarza nad funkcją - usunięcia przedimka 'the'. Tylko tyle. Ale to pozwoliło mi usunąć coś, co mi nie pasowało, i trochę rozgościć się na nowym terenie. Znaleźć ten pierwszy przyczółek, z którego można zacząć abordaż.

W drugim commicie poprzenosiłem funkcje, tak aby te jednego typu były blisko siebie - zmniejszając pomiędzy nimi odległość pionową, grupując funkcje wzajemnie się wywołujące, więc czytane razem. W trzecim commicie przekształciłem funkcję na 'pure function', usuwając z niej zmienną globalną. Później posortowałem alfabetycznie funkcje i tak dalej.

Niektóre zmiany zostały później wprowadzone, ale zdecydowana większość nie.

Te zmiany niekoniecznie muszą być ostatecznie wprowadzone. Przyznam, że jeśli mam bardzo skomplikowany kod, którego nie znam, to wprowadzam do niego bardzo dużo zmian podczas czytania kodu - zmieniając formatowanie, zmieniając nazwy elementów systemu, które niekoniecznie są poprawniejsze. To decyzje podejmowane przy pierwszych nieśmiałych próbach poznania i zrozumienia systemu, a nie na podstawie jego głębokiej znajomości. Zapewne te nowe nazwy zmiennych w całym kontekście mogą nie być lepsze, ale w tym konkretnym przypadku, kiedy zaczynamy czytać, pozwalają lepiej zrozumieć, zbudować sobie pewien obraz, model mentalny, z jakich elementów składa się aplikacja i jak działa. Chodzi o to, aby ruszyć z miejsca.

Często miałem tak, że bardzo długo czytałem kod, próbując go zrozumieć i jego powiązania, ale nic absolutnie nie zmieniałem, bo się po prostu bałem. To jednak kompletnie utrudnia zrozumienie. Postawienie danego obszaru do testów byłoby niezwykle trudne i czasochłonne. Nie czekajmy aż poznamy i zrozumiemy kod na tyle dobrze, że będziemy mieli pewność co do wprowadzanej zmiany. Po to mamy system kontroli wersji Git, abyśmy nie obawiali się zmian.

To podobnie jak z rysowaniem. Rysunek wyłania się z drobnych szkiców, ołówkiem, nie stawiamy od razu głównych linii, które są perfekcyjne. Te linie główne, właściwe, wyłonią się same z kilkudziesięciu innych pociągnięć ołówkiem.

7. jak duże ryzyko wnosi ta zmiana?

Korzystając z oznaczania danego typu brakowało mi jeszcze jednej rzeczy. Jako, że głównie pracuję z 'legacy code'. Patrząc na historię typ commita określa na które zmiany warto zwrócić uwagę, a które możemy pominąć w przeglądaniu. Rzeczy które zawsze możemy pominąć to np. te dotyczące stylu, czyli zmiana formatowania, usunięcie whitespace itp. Dlaczego możemy je pominąć? Ponieważ nie zmieniają zachowania kodu, ani nie ingerują w jego strukturę jak np. refaktoring. Więc tym samym są zmianą bezpieczną czyli taką która nie wnosi żadnego ryzyka. Ryzyko to właśnie to czego mi brakowało.

Które zmiany są na tyle ryzykowne, że powinny zwrócić uwagę? Samo wymuszenie ustalenia ryzyka, próby przypisania i określenia jakie ryzyko wnosi dana zmiana powoduję, że musisz się nad tym zastanowić. Jeśli sądzisz, że ryzyko zmiany jest duże, może warto je zmniejszyć dodając testy albo dzieląc zmiany na mniejsze fragmenty. To wprowadza dodatkowy checkpoint do zadumy i zastanowienia się nad intencja i ryzykiem zmiany.

Znalazłem już istniejącą notację Arlo's Commit Notation ale uważam, że jej zapis jest trochę enigmatyczny bo do oceny ryzyka używa liter i znaków interpunkcyjnych. A sadzę, że do oceny ryzyka idealnie nadają się cyfry od 0 do 3 bo od razu widać gdzie ryzyko jest większe bądź mniejsze. Poniżej próba opisania możliwych ryzyk z notacji Arlo:

Risk Number Risk Level Meaning Correctness Guarantees
0 Known safe Addresses all known and unknown risks. Intended Change, Known Invariants, Unknown Invariants
1 Validated Addresses all known risks. Intended Change, Known Invariants
2 Risky Some known risks remain unverified. Intended Change
3 (Probably) Broken No risk attestation.  
  • 0 - daję z automatu każdej zmianie dotyczące stylu [style] oraz testom [test].
  • 1 - większość rzeczy które nie mają testów, więc to najpopularniejsze ryzko
  • 2 - obowiązkowo daje każdej migracji
  • 3 - czasem zdarza mi się, że jestem w trakcie pracy i muszę ją przerwać, więc szybko daję 3 i piszę 'WIP' abym np. następnego dnia do tego zasiadł był świadomy, że zostawiłem stan niedziałający

Przykładowa historia commitów z projektu który był stawiany od nowa - to proste API:

0 [feat][test] testMeetingShouldNotBeLongerThanEightHours
0 [refactor] CustomKernelTestCase->createUser - provide default values
0 [test] testMeetingOwnerShouldAlwaysBeThePersonWhoCreateTheMeeting
1 [feat] User has a password - authorization
1 [refactor] Meeting owner req in constructor
0 [refactor] fix Meeting relations with User
0 [refactor][rename] MeetingTest -> MeetingListingTest
0 [test] add testSearchByRoomName
0 [refactor] createMeeting now takes Room params for input
0 [test] add testShowDetailsOfRoomWhenListingMeetings
0 [test][fix] add RoomFactory
1 [feat] add Entity\Room
0 [test] add testSearchByStartDay
0 [refactor] MeetingTest, add CustomKernelTestCase
0 [test] add testSearchMeetingByAgenda
1 [test] MeetingTest update 
0 [chore] composer req zenstruck/browser
1 [fix] Meeting.owner not ownerId, should be ManyToOne
1 [chore] add phpunit, first MeetingTest
1 [feat] add Entity/Meeting 
0 [remove] dir /migrations
1 [chore] composer req foundry orm-fixtures --dev
0 [docs] User api annotation
0 [chore] composer req debug
2 [feat][migration] add User entity
0 [chore] composer req maker --dev
0 [chore] composer req api
0 [chore] symfony new
Add initial set of files

8. podsumowanie

Wprowadzenie notacji commitów opartej na dwóch pytaniach - "jakiego typu jest ta zmiana?" i "jak duże ryzyko wnosi?" - oraz dostosowanie stylu integracji kodu do tych wytycznych okazało się przełomem w moim codziennym podejściu do pracy.

Mam nadzieję, że przedstawione podejście pomoże Wam w codziennej pracy i uczyni ją bardziej efektywną oraz satysfakcjonującą. Zachęcam do eksperymentowania z różnymi metodami i dostosowywania ich do specyfiki Waszych projektów oraz zespołów. Powodzenia!

9. polecane artykuły

10. słownik

  • używam terminu Merge Request (MR)
    • Pull Request (PR) - oznacza to samo tylko jest używany w GitHub i Bitbucket a MR na GitLabie
  • używam zamiennie terminów 'commit' i 'zmiana/y'
  • używam zamiennie terminów 'branch' i 'gałąź'
  • integracja to merge do master'a

Date: 2024-06-16 Sun 00:00

Author: Slawomir Grochowski

Email: slawomir.grochowski@gmail.com

Created: 2024-06-17 Mon 08:53