Code Review z Phabricatorem i Arcanistem
Nowa fabryka, nowy projekt, nowe wyzwania. Tak można by podsumować ostatnie trzy tygodnie w moim wykonaniu. Projekt został przejęty po innej firmie, która się zwinęła z rynku. Otrzymaliśmy niewydolne repozytorium svn z pełną historią, dokumentację i tyle. Kilka osób w zespole przeszło z tamtej firmy, ale jest to dla nich stan przejściowy. Co najważniejsze nie mamy, poza kodem, nic innego. Trzeba wszystko zestawić praktycznie od zera.
Dziś chciałbym opisać nasz pomysł na code review. Jednak zanim do tego przejdę, to krótkie wprowadzenie, które przybliży, dlaczego wybraliśmy takie, a nie inne rozwiązania.
Dlaczego Phabricator?
Jak już wspomniałem, mamy kod, ale nie mamy żadnego narzędzia, które pozwoliłoby nam na organizację procesów. Nie mamy też budżetu na zakup JIRY, a
charakter projektu, bankowy core, wyklucza wypchnięcie całości na rozwiązanie chmurowe w rodzaju Githuba czy Bitbucketa. Poszukiwaliśmy zatem
rozwiązania opensource, bezpłatnego w opcji self-hostingu, oraz kompletnego. Kompletność oznacza, że w jedynym narzędziu, albo grupie narzędzi
współpracujących ze sobą, mam dostęp do wszystkich elementów związanych z prowadzeniem projektu. Tak właśnie działa JIRA z przyległościami. Grupa
narzędzi, które możne ze sobą łatwo zintegrować. Jednak JIRA jest poza zasięgiem budżetu. Po krótkim rozpoznaniu wybór padł
na Phabricatora.
Phabricator jest otwarty, bezpłatny i zawiera wszystko, czego potrzebujemy. Jest obsługa zgłoszeń, wiki, kanban board, zarządzanie repozytoriami kodu,
CI oraz narzędzia do code review. Całość napisana w php, co oznacza, że wystarczy bardzo prosta konfiguracja po stronie serwera. Do tego całość jest
żywa, to znaczy aktywnie rozwijana, oraz sprawdzona przez większych graczy np. Ubera, Facebooka, czy Fundację Wikimedia. Proces instalacji i
konfiguracji jest bardzo prosty i dobrze opisany w dokumentacji. Aktualizacja do nowszej wersji polega na zatrzymaniu serwisów, zrobieniu git pull
i
ponownym uruchomieniu serwisów.
Przejdźmy zatem do dania głównego tego wpisu, czyli narzędzia do code review.
Rodzaje Code Review
Przegląd kodu można prowadzić na wiele sposobów. Każdy z nich ma zalety i wady. Każdy może też być dobrze i źle wdrożony. Nie twierdzę, że nasz pomysł jest najlepszy z możliwych, ale na chwilę obecną wydaje się najsensowniejszy. Przyjrzyjmy się jakie mamy możliwości i czym się różnią.
Przeglądy post-commit
Jak sama nazwa wskazuje przeglądy takie, są wykonywane na kodzie, który już znajduje się w repozytorium. Typowy proces pracy wgląda w tym przypadku w następujący sposób:
- Anna commituje zmiany do repozytorium. Najczęściej na osobną gałąź.
- Następnie tworzy CR request, czyli w narzędziu oznacza commit albo grupę commitów jako gotowe do przeglądu.
- Mike wykonuje przegląd, dodają uwagi.
- Anna wprowadza poprawki i rozszerza CR o kolejny commit. Rozszerzenie może dziać się automatycznie.
- Mike akceptuje poprawki i zamyka CR
W tym momencie możliwe są dwa scenariusze. Zamknięcie CR automatycznie łączy gałąź ze zmianami z gałęzią główną. Warunkiem koniecznym jest możliwość
złączenia bez konfliktów. Drugi scenariusz to ręczne łączenie gałęzi.
Takie podejście jest o tyle fajne, że jest bardzo proste w implementacji. Zazwyczaj nie wymaga żadnych dodatkowych narzędzi poza narzędziem do CR.
Całość opiera się na poleceniach naszego VCS-a. W dodatku jesteśmy niezależni od silnika VCS. Wady są dwie. Po pierwsze przeglądany kod jest już w
repozytorium. Po drugie nie wszystkie silniki VCS są, w stanie płynnie działać w ten sposób. Chyba najlepszym przykładem jest SVN i jego model
tworzenia gałęzi. Dla dużej ilości danych będzie to powolne, a ciągły schemat nadawania numerów wersji będzie tylko przysparzał problemów. W dodatku
nie mamy możliwości zablokowania commitowania do głównej gałęzi. Podobnie w git. Co prawda można tak skonfigurować narzędzia, by nie akceptowały
commitów bezpośrednio do mastera i pozwalały jedynie na pull requesty, ale na „czystym” gicie jest to, co najmniej uciążliwe.
Audyty
W praktyce podejście post-commit nie służy do CR rozumianego jako element wewnętrznego procesu w zespole. Podejście to służy do audytu kodu przez
podmiot zewnętrzny. Załóżmy, że dowieźliśmy sprint na czas. Jednym z artefaktów końcowych, jest opis zmian w kodzie w formacie podobnym do tego,
którego używamy do CR. Nasz klient wysyła taki pakiet do na przykład usługi Veracode, gdzie zmiany są badane pod kątem luk bezpieczeństwa. Inaczej
mówiąc, wszystkie zamiany wprowadzone przez zespół podlegają przeglądowi przez niezależny podmiot.
Phabricator wspiera ten model i co ciekawe jest to osobny moduł. Audyty są czymś niezależnym od CR. Jest to osobne menu.
Pull request
Wspomniałem wyżej, że w gicie można wykorzystać trochę inny mechanizm, a mianowicie pull requesty. Schemat działania jest podobny do poprzedniego. Też
trzeba zacommitować zmiany do gałęzi w zdalnym repo (bare), a następnie wypchnąć zmiany do własnego repozytorium. Kolejnym krokiem jest stworzenie
PR-ki do repozytorium docelowego. I działa to świetnie. PR-ka może zostać przejrzana i zaakceptowana bądź odrzucona. Kolejne zmiany w naszym
repozytorium będą uwzględniane w PR-ce we w miarę zautomatyzowany sposób. Oczywiście o ile korzystamy z jakiegoś narzędzia w rodzaju githuba.
Zresztą jest to najpopularniejsza metoda pracy z repozytoriami na githubie. Ma jednak pewien feler. W pewnym momencie mamy dwa repozytoria, które mają
w głównych liniach kodu dwie różne wersje kodu. Można to naprawić. Wystawiając PR gałęzi z naszego repozytorium do głównej gałęzi repozytorium
docelowego… co efektywnie można zredukować do commitowania do gałęzi w repozytorium docelowym i w efekcie PR staje się takim post-commit CR, tyle
tylko, że jest ładniej nazwane. Dodatkową wadą jest ograniczenie się do rozproszonych VCS-ów. Jeżeli chcemy zrobić coś takiego w SVNie, to musimy
korzystać z mirroringu i personal upstreamów, które są zwyczajnym hakiem.
Przeglądy pre-commit
I tu dochodzimy do przeglądów wykonywanych przed commitem. Przy czym „przed commitem” oznacza, że zmiana nie wycieka poza nasze lokalne repozytorium.
Na czym to polega. Zamiast wysyłać zmiany do zdalnego repozytorium, przygotowujemy patch file, który będzie podlegać przeglądowi. Taki patch file jest
traktowany jako wsad do narzędzia CR. Akceptacja zmian pozwala nam na ich opublikowanie w zdalnym repozytorium.
Taki model jest przyjęty wśród programistów jądra. Linus ostatecznie akceptuje lub odrzuca zmianę. Zanim zmiana trafi do niego, jest oczywiście
przeglądana przez opiekunów poszczególnych modułów, a oni otrzymują wstępnie przesiane zmiany przez opiekunów poszczególnych plików lub fragmentów
kodu. Ci są adresatami zmian, które są tworzone przez zwykłych programistów. Oczywiście opisuję tu model w sposób uproszczony, ale to mniej więcej tak
działa. Zaletą tego modelu jest to, że zmiana nie jest opublikowana przed akceptacją. W dodatku całość jest niezależna od systemu VCS, ponieważ opiera
się na patch filach, które są standardem. Najpoważniejszą wadą jest użycie formatu patch file, który ma pewne ograniczenia, m.in. w przeciwieństwie do
PR nie można ich budować ze zmian, które mają różnych autorów. Drugą wadą jest konieczność przestrzegania procesu, bo narzędzia nie radzą sobie
jeszcze z np. commitem bez zatwierdzenia CR. Tu na scenie pojawia się phabricator i jego narzędzie arcanist.
Dlaczego nie gerrit?
Tu dochodzimy do kolejnego pytania. Dlaczego nie użyć gerrita? Z bardzo prostej przyczyny. Gerrit pracuje tylko z repozytoriami git, a obsługa svn jest możliwa tylko poprzez git-svn. Tyle tylko, że git-svn wymaga gita, a my nie możemy go użyć w jednym z naszych projektów. W dodatku jest to kolejne narzędzie, które wymaga uwagi z naszej strony.
Dlatego arcanist
Arcanist jest konsolowym interfejsem phabricatora. Daje on dostęp do różnych modułów, ale my skupimy się na module differential, który służy do zarządzania CR.
Mała uwaga, w dokumentacji Differentiala jest mowa o pre-push CR. Wynika to z popularności gita i gitowej nomenklatury. W przypadku svn-a jest to oczywiście pre-commit.
Instalacja i konfiguracja projektu
Żeby móc korzystać z arcanista należy go zainstalować. W przypadku linuxów znajduje się on w pakiecie arcanist
, który wymaga instalacji php7.
Dodatkowo należy zainstalować najnowszą wersję pakietu php7.0-xml
/php7.1-xml
. Pakiet ten nie jest w podstawowej dystrybucji php i trzeba go
samodzielnie zainstalować.
Po zainstalowaniu arcanista należy jeszcze skonfigurować projekt. Konfiguracja jest bardzo prosta. W katalogu głównym należy utworzyć
plik .arcconfig
, który jest prostym JSON-em:
Listing 1. Plik .arcconfig w wersji minimum.
{
"phabricator.uri": "http://phabricator.yourcompany.pl/",
"repository.callsign": "NAZWAREPO"
}
Zgodnie z dokumentacją minimalna konfiguracja może być ograniczona tylko do własności phabricator.uri
, ale bez repository.callsign
arcanist może
mieć problemy z rozpoznaniem repozytorium projektu. Teoretycznie robi to automatycznie, ale nie zawsze to wychodzi. W szczególności, gdy nazwa lokalna
różni się od zdalnej.
Tworzenie CR
Proces tworzenia CR jest niezależny od silnika VCS, którego używamy. Phabricator wspiera git-a, svn-a i mercicurala więc nie jest źle. Po wprowadzeniu zmian w kodzie wystarczy w linii poleceń uruchomić:
Listing 2. Tworzenie CR
$ arc diff
Zostanie uruchomiony edytor, w moim przypadku jest to nano, w którym musimy wypełnić szablon z informacjami:
Listing 3. Szablon CR
<<Replace this line with your revision title>>
Summary:
Test Plan:
Reviewers:
Subscribers:
# NEW DIFFERENTIAL REVISION
# Describe the changes in this new revision.
#
# arc could not identify any existing revision in your working copy.
# If you intended to update an existing revision, use:
#
# $ arc diff --update <revision>
Obowiązkowymi polami są tytuł, podsumowanie i plan testów. Pola Reviewers i Subscribers są opcjonalne i podajemy tam loginy. Po zapisaniu i wyjściu z edytora arcanist utworzy CR w phabricatorze. W tym celu używa crul-a.
Jeżeli pracujemy z gitem, to nie musimy wykonywać git commit, bo arc diff zrobi to za nas. W przypadku svn-a zostanie wygenerowany patch file i nie
będzie zmian w repozytorium. Pytanie, które zmiany są brane do CR? Odpowiedź jak zawsze brzmi, to zależy. W przypadku svn-a są to zmiany, które
są widoczne w svn status
. W przypadku git-a są to zmiany widoczne w po zaaplikowaniu git merge-base origin/master ..HEAD
. Inaczej mówiąc, wszystkie
różnice pomiędzy aktualną gałęzią a zdalnym masterem.
Akceptacja CR i różnice pomiędzy VCS-ami
Jeżeli chcemy zobaczyć, jakie CR czekają na nas, wystarczy wydać polecenie arc list
.
Listing 4. Lista oczekujących CR
$ arc list
*Needs Review D6:Testowe CR w svn
Należy tu zwrócić uwagę na identyfikator CR. W tym przypadku jest to D6
. Będzie on nam potrzebny do dalszej pracy z arcanistem. Jeżeli CR-ka
zostanie zaakceptowana, to wystarczy klepnąć zmianę i wysłać ją do repozytorium. Niestety nie ma jednego spójnego interfejsu do wykonania tej
operacji.
Listing 5. Zamknięcie niezaakceptowanego CR w svn-ie
$ arc commit--revision D6
Revision'D6: Testowe CR w svn'has not been accepted.Commit this
revision anyway?[y/N]N
Usage Exception:User aborted the workflow.
Oczywiście można wymusić zamknięcie CR i wysłać zmiany do repozytorium.
Listing 6. Zamknięcie niezaakceptowanego CR w git
$ arc land--revision d7
Landing current branch'master'.
TARGET Landing onto"master",selected by following tracking branches upstream to the closest remote.
REMOTE Using remote"origin",selected by following tracking branches upstream to the closest remote.
FETCH Fetching origin/master...
This commit will be landed:
-4affd51 OK
Revision'D7: OK'has not been accepted.Continue anyway?[y/N]
Usage Exception:User aborted the workflow.
Jak widać svn i git mają osobne polecenia. To nie jest najfajniejsze rozwiązanie, ale daje radę. Jeżeli CR zostało zaakceptowane, to nie otrzymamy komunikatu, że coś wisi, tylko zmiana zostanie wysłana na serwer. Tu właśnie wychodzi największa zaleta modelu pre-commit/pre-push w porównaniu do pull requestów. Zgłoszenie CR jest nieblokujące. Oczywiście, jeżeli ktoś odwali manianę i wyśle kod, który nie był przejrzany, to odpowiednia adnotacja pojawi się w historii CR. Winny zostanie już odpowiednio nagrodzony…
Za i przeciw
Czas na krótkie podsumowanie. Phabricator ma bardzo dobrze przemyślany mechanizm przeglądów kodu. W dodatku jasno rozgranicza przeglądy od audytów.
Wspiera kilka różnych rodzajów repozytoriów. Co prawda interfejs nie jest spójny, ale różnice są minimalne. Całość jest nieblokująca i w
przeciwieństwie do klasycznych gitowych PR, łatwiejsza do zrozumienia.
Rozwiązanie nie jest pozbawione wad. Największą jest konieczność używania arcanista, który jest dodatkowym oprogramowaniem. Jeżeli używamy
niestandardowego sposobu zarządzania commitami, to musimy zapomnieć o automatyzacji niektórych czynności. Gotowe rozwiązania muszą zostać dostosowane.
Niektórych może też zaboleć, że całość jest w php, co powoduje, że kod jest rozszerzalny dla zespołów niephpowych.
Podsumowując, lubię i polecam. Pobawić się można, a potencjalne korzyści są duże. Jeżeli zatem szukasz narzędzia do ogarniania projektu, to phabricator będzie całkiem fajny.