Sealhub Workflow
Przedstawiam poniżej propozycję sposobu pracy z Sealhubem (napędzanym przez Phabricatora).
Audyt vs Review
Phabricator zawiera dwie duże funkcjonalności umożliwiające wzajemne "przeglądanie kodu" przez pracujących nad nim programistów: Audyty i Review. O ile mogą się one zdawać na pierwszy rzut oka funkcjonalnie tożsame, każda z nich ma swoje unikalne cechy, które sprawiają, że pasuje ona lepiej do niektórych scenariuszy, niż ta druga.
W telegraficznym skrócie:
kiedy się wykonuje | dotyczy | ile czasu potrzeba | rozmiar zmian | przykład | |
review | przed pushem | wielu commitów | więcej | duże, funkcjonalne zmiany, refactory, dokumentacja | Implementuję nowy widok w aplikacji webowej. Nowa logika, style. |
audyt | po pushu | zawsze pojedynczego commita | mniej | szybkie fixy, jednolinijkowe zmiany, trywialne commity, bezdyskusyjne zmiany | Klient zgłosił poprawki do CSS-a i prosi o szybki deploy |
Warto też przeczytać, co na temat tego rozróżnienia napisali twórcy Phabricatora: https://secure.phabricator.com/book/phabricator/article/reviews_vs_audit/
Audyt
Audyt tyczy się pojedynczego commita i zachodzi po wysłaniu commita na repozytorium. Dany commit może mieć stan "wymaga audytu" lub "nie wymaga audytu".
Powyższy zrzut ekranu przedstawia część widoku repozytorium DlaSchroniska w webowej przeglądarce repozytoriów. Widzimy na nim trzy commmity, (po kolei, od góry):
- pierwszy z nich nigdy nie wymagał audytu,
- drugi z nich wymagał audytu i dostał pozytywną opinię (czyli już nie wymaga audytu),
- trzeci z nich wymagał audytu i ktoś zgłosił pewne obawy, którymi należy się zająć (czyli wciąż proces audytu nie jest zakończony).
Jeżeli ktoś zgłosi w trakcie audytu potrzebę zmian, należy stworzyć i spuszować osobny commit, który poprawia te zmiany.
Aby sprawić, że commit wymaga audytu, należy dodać osobę do listy audytorów.
W trakcie audytu można nanosić komentarze "inline" do zmian z danego commita, ale nie można wprowadzać już zmian w wysłanym commicie, można tylko wysłać nowy commit, który naprawia coś z danego commita. Jest to istotna różnica w porównaniu z Review.
Za pomocą narzędzia Herald można stworzyć reguły, które automatycznie dopisują kogoś do listy audytorów, jeżeli dany commit spełnia pewne konfigurowalne warunki (np. "Dodawaj Arkadiusza do listy audytorów commitów, które zmieniają coś w CSS.").
Review
(będe używał słów "Review" i "recenzja" zamiennie, z powodów stylistycznych)
Review tyczy się większych zmian, wymagających konsultacji z innymi członkami zespołu. Review zachodzi przed wysłaniem zmian na repozytorium. Prosty przykładowy flow prezentuje się następująco:
- Deweloper A otwiera feature-branch na lokalnym repozytorium;
- Deweloper A tworzy kilka commitów;
- Deweloper A decyduje, że nowa funkcjonalność jest gotowa i chce poddać ją review. Tworzy więc diffa swojego feature-brancha z masterem i umieszcza go w Differential, np. korzystając z interfejsu webowego;
- Deweloperzy B i C umieszczają komentarze w diffie i sugerują jakieś zmiany;
- Deweloper A nanosi swoje zmiany w lokalnym repozytorium, tworzy nowego diffa i wkleja go w interfejsie webowym jako nową wersję poprzedniego diffa (a nie jako nowego diffa); //kroki 4 i 5 powtarzają się tak długo, aż zespół jednogłośnie akceptuje diffa
- Następuje tzw. "land". Deweloper A squashuje swoje commity z brancha w jeden i dokleja go jako pojedynczy commit w defaultowym branchu, a następnie pushuje go.
(Oczywiście nikt nie musi ręcnie generować diffow, przeklejać ich ręcznie z terminala do interfejsu webowego ani wiedzieć, jak się squashuje commity - istnieje narzędzie CLI, które robi to automatycznie: Arcanist. Możesz dowiedzieć się o nim więcej kilka paragrafów poniżej).
Review stanowi pewne zaprzeczenie znanemu już GitHub Workflow, gdyż flow zamiast tak:
write -> publish -> review -> merge
wygląda tak:
write -> review -> merge -> publish
Zalety płynące z Review to, m.in:
- na repozytorium nie lądują nieprzejrzane commity - zmniejsza się drastycznie prawdopodobieństwo potrzeby revertów lub commitów naprawiających;
- zachęca autorów do tworzenia możliwie małych, zrozumiałych zmian;
- daje recenzentom okazję do sugerowania istotnych zmian w kodzie, co jest trudne przy np. robieniu audytu commita, który wylądował na repozytorium 15 commitów temu;
- autor diffa może poprosić recenzentów o wprowadzenie poprawek;
- zespół jest wdrożony w nowe zmiany w projekcie - na produkcji nie ląduje kod, którego wcześniej nie widzieli;
- motywuje autora diffa to reagowania na zmiany - ponieważ review blokuje zmiany z negatywną recenzją;
- sprawia, że historia commitów wygląda czytelniej i ma bardziej płaską strukturę - każdy commit tyczy się pojedynczej, izolowanej zmiany i ma tytuł niosący za sobą znaczenie (Arcanist pozwala na automatyczne generowanie tytułów commitów na podstawie danych z Review). Przykład repozytorium rozwijanego wg Review Workflow:
phabricator/ $ git log --graph --oneline * 3f50ba9 On workboards, put older milestone columns on the right * 9bab3c2 Sort milestones by milestone number, not ID * bec21d8 Don't try to import proxy columns * 1e3a5bd Filter out archived projects from ProjectProfileView * de4167d Put boundary spaces around crumb names so double-clicking doesn't flip out * ca908d7 Don't autoname milestones, but do show the previous milestone name as a hint * 68d0593 Don't show un-completeable results in people/project autocomplete * 2f054ed Hide milestone columns when milestone is archived * a5bbe25 Fix a couple of missing translation strings ... * 01084bf Begin making card updates on boards independent of context * 9ed9764 Replace height buffer behavior while dragging on workboards with infinite column height * bc591b1 Mostly move workboard card moves to new Workboard code ...
Przykład z repozytorium rozwijanego wg Github Workflow:
rails/ $ git log --oneline --graph * b635eea use kwargs to avoid hash slicing * 8c53b41 Merge pull request #23611 from abhishekjain16/routes_options |\ | * 4e3931a Fixes routes to match verbs and path with -g option * | da1fbb9 Add fixes accidentally removed. * | 354fb73 Flesh out request encoding + response parsing changelog entry. * | d50d709 Merge pull request #23642 from tvon/chore/fix-path-omission |\ \ | * | 1a61496 Use correct path in documentation. * | | db64cba Merge pull request #23641 from abhishekjain16/docs_fix |\ \ \ | |/ / |/| | | * | 9a2ca9c [ci skip] Fix enqueuing spelling to maintain consistency |/ / * | c63f58d Merge pull request #23639 from Gaurav2728/update_change_log_entry_action_pack |\ \ | * | 33e202d use rails instead of rake |/ / ... | |/ / / / / / / / |/| | | | | | | | | * | | | | | | | 03980b2 Converting backtrace to strings before calling set_backtrace * | | | | | | | | 2db347b Merge pull request #23395 from PareshGupta/remove-unused-constant |\ \ \ \ \ \ \ \ \ ...
Zachęcam do lektury: https://secure.phabricator.com/phame/post/view/766/write_review_merge_publish_phabricator_review_workflow/
Arcanist
Arcanist to narzędzie umożliwiające wygodne nawigowanie po Review Workflow z poziomu linii komend. Potrafi m.in.:
- tworzyć diffy i wysyłać je na serwer (arc diff);
- aktualizować diffy (arc diff);
- finalizować diffy, squashować commity i wysyłać je na repozytorium (tzw. "land" - arc land);
Aby zacząć korzystać z Arcanista, zainstaluj go (powinien być w domyślnych repozytoriach Twojego systemu - w razie potrzeby więcej info tutaj. Następnie, aby powiązać dane repozytorium z instancją Phabricatora (nasza instancja stoi na https://hub.sealcode.org), dodaj plik o nazwie .arcconfig w korzeniu repozytorium (jeżeli jeszcze go nie ma), o następującej zawartości:
{ "phabricator.uri" : "https://hub.sealcode.org/" }
A nastepnie wykonaj polecenie
# arc install-certificate
i podążaj za instrukcjami. Jest to jednorazowy krok mający na celu uwierzytelnienie Arcanista na Twojej maszynie w danym repozytorium lokalnym.
Aby wysłać zmiany do review, wpisz:
# arc diff
i podążaj za instrukcjami na ekranie.
Aby zrobić "land", tzn. zrobić z całego diffa jeden commit i dodać go do domyślnego brancha, wpisz:
# arc land
Wybór domyślnego edytora
Arcanist korzysta z domyślnego edytora, aby umożliwić konfigurację diffów (wliczając w to tytuły diffów i listę recenzentów). Aby wybrać domyślny edytor, ustaw odpowiednio zmienną środowiskową $EDITOR (prawdopodobnie możesz to zrobić w Twoim pliku .bashrc).
Kilka review naraz
Może zdarzyć się, że wyślesz zmiany na review i pozostaje Ci czekać, aż ktoś je zrecenzuje. W międzyczasie możesz oczywiście pracować nad innym diffem! Załóż nowego brancha, który ma zwój korzeń w masterze, wykonaj kilka commitów i ponownie możesz założyć nowy review request za pomocą polecenia arc diff.
Kiedy korzystać z Review, a kiedy z Audytów?
Mając na uwadze plusy i minusy tych rozwiązań zależnie od scenariusza proponuję, aby w projektach stosować obydwa podejścia - zależnie od charakteru taska. Część tasków wymaga solidnych dyskusji i konsultacji z zespołem (nowe widoki, zmiany w API itp bardziej proszą się o Review, a szybkie fixy o proste audyty)
Kwestia tego, które taski są "nagłe trywialne" i zasługują tylko na audyt, a które są większe i wymagają review jest dosyć subiektywna i uważam, że lepiej nie pozostawiać takiej decyzji osobie odpowiedzialnej za taska (bo kuszące jest pomyśleć "e tam, to są trywialne zmiany, ja je rozumiem i audyt w zupełności wystarczy"). Myślę, że warto zdecydować w zespole, które taski są realizowane w trybie Review, a które w trybie Audytu. Korzystajc z faktu, ze w Pahbricatorze można tworzyć własne tagi do tasków, dodałem dwa: "Tryb Review" i "Tryb Audytu". W trakcie spotkań zespołu można synchronicznie ustalić, w jakim trybie realizowane są taski.
Nazewnictwo commitow
Język commitów
Zależy od projektu. Projekty open-source dużo zyskują, jeżeli mają tytuły pisane po angielsku. Wszechobecnym standardem w takim wypadku jest pisanie commitów w trybie imperatywnym (rozkazującym), zaczynając wielką literą:
Add new amazing feature
Projekty zamknięte mogą zyskać z pisania commitów w języku polskim - jest większa jednoznaczność przekazu.
Dodano nową zajebistą funkcję
Tagowanie commitów
W Phabricatorze (a więc i w Sealhubie) można przypinać do commitów taski. Jeżeli np. do tytułu commita dopiszemy "Ref T28", to w widoku commita pojawi się link do taska, a także w widoku taska pojawi się ten commit.
Zrobiłem dla testu podpiętego commita w testowym repo:
https://hub.sealcode.org/rPGbaf5a544352cda1edf4b99ec9a24e6b170f6490e
Można zobaczyć, że podpiął się do T28:
- Last Author
- arkadiusz-wieczorek
- Last Edited
- Feb 22 2017, 16:07