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 chwile 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ą, wstanie 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 jest 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 aktualna 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 maniane 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.

3 myśli na temat “Code Review z Phabricatorem i Arcanistem

  1. Przyglądałeś się Gitlab CE? Bardzo prężnie się rozwija i już dużo potrafi.

  2. Nie znałem. Podoba mi się, sprawdzę dokładniej.
    Dzięki!

Napisz odpowiedź

Twój adres email nie zostanie opublikowany. Pola, których wypełnienie jest wymagane, są oznaczone symbolem *

To create code blocks or other preformatted text, indent by four spaces:

    This will be displayed in a monospaced font. The first four 
    spaces will be stripped off, but all other whitespace
    will be preserved.
    
    Markdown is turned off in code blocks:
     [This is not a link](http://example.com)

To create not a block, but an inline code span, use backticks:

Here is some inline `code`.

For more help see http://daringfireball.net/projects/markdown/syntax