Agenda:

Pytajcie w trakcie na chacie - odpowiemy async po skończonych sekcjach

Agenda cd.

Powtarzające się uwagi

DEMO

Stylecop (prettier.io, checkstyle)

Stylecop cd.

PrefixLocalCallsWithThis - przykład że coś trzeba wybrać i się stosować.

... w dowolną stronę (z thisem, albo bez thisa)

Uniwersalne narzędzie do formatowania

prettier.io

Kolega ustawił indentation na 2 spacje - nigdy w życiu nie miałem takiego ustawienia. Ale dzięki temu że jest to wymuszone to nie narzekam i się przyzwyczaiłem (chociaż przeformatowało cały projekt)

Testy konwencji

3 kawałki:

• pętla po wszystkich plikach w projekcie
• szukanie problemów w jednym pliku
• odpalenie tych testów podczas buildu

Przykład: wymóg aby wszystkie kolekcje były niemutowalne w C# (IReadOnlyCollection<>)

Powtarzające się uwagi

• automatyzować (jeśli się da)
• może nie reagować

Jak unikać flame wars

tabs vs spaces

Wy sami wiecie co odpuścić. Zachęcam żeby czasem odpuszczać.

TLDR - za długie CR czy może coś, co trzeba bez CR "przepchnąć"?

• olbrzymi rename - nikt nie chce tego przeglądać
• olbrzymi rename i gdzieś logika się zmieniła
• 

Oczyszczenie commitu (commitów) przed CR

• Fix typo
• Poprawa formatowania
• Zmiana namespace'u / folderu / pakietu

Jak namawiać do braku CR?

Przykłady gdzie warto się zastanowić

• jakiś projekt na wymarciu gdzie nie chcemy inwestować swojego czasu
• 1 osoba w nowym projekcie i nikt nie kwapi się do przeglądania (jeśli chodzi o dzienne review)
• ... !!! ale z pewnością co jakiś czas ktoś powinien doglądać w którą stronę to ogólnie idzie
• 2 osoby które ze sobą rozmawiają
• jeśli mam przeglądać ogromny MR TechLeada, który zmienia coś we frameworku (co mnie nie dotyczy)

Więc jak namawiać do braku CR?

Przemyśleć sobie czy da nam to więcej zysku niż zainwestowany w to czas.

• zakładam że proces jest w rękakach zespołu i możemy to sobie zmienić
• ... albo robić wyjątki
• może się okazać, że po pewnym czasie zespół się dociera i już na CR nie są wyłapytawne kardynalne błędy
• ... decyzje odnośnie architektury są wystarczająco dobrze przegadane przy kawach i kod powstaje w odpowiednich miejscach

Konieczny refactoring w trakcie feature'a - jak to reviewowac?

DEMO

Porównanie dwóch podejść

Porównanie dwóch podejść cd.

Od startu projektu do code review

Wybór flow commitów

☺ Jeden branch długo-żyjący

☹ Wiele branchy długo-żyjących

Wybór flow dołączania feature brancha do core brancha

• Basic merge
• Squash merge
• Rebase and fast-forward
• Rebase with merge commit

Basic merge

Da się wyciągnąć informację z historii, ale jest to trudne, czasami bardzo trudne.

Squash merge

Gdy feature branche są bardzo małe, da się coś wyciagnąć z historii, ale stosunkowo niewiele.
Ważna wiedza spoczywa poza repo - w zewnętrznym narzędziu.

Rebase and fast-forward

Historia jest przejrzysta, można wyciągnąć z niej bardzo dużo.
Tracimy informacje o pullrequestach i 'zarys', które commity były w ramach jednego feature brancha.

Rebase with merge commit

Historia jest przejrzysta, można wyciągnąć z niej wszystko.

Wybór polityki ochronnej dla core brancha

Blokada bezpośrednich pushy do core brancha

Wszelkie zmiany możliwe tylko przez PR.

Polecane nawet, gdy pracujesz z samymi seniorami-wymiataczami,
a nawet gdy prowadzisz dany projekt samemu.

Branch policy - sposób dołączania feature brancha

Branch policy - build pipeline

Branch policy - required approve

Minimum 1 wymagany approve (ale nie kliknięty przez siebie samego).
Ja polecam: wymagany 1 approve od kogoś z grupy Developers i 1 approve od kogoś z grupy Testers.

Branch policy - additional approve for specific project parts

Gdy zmienione są pliki ze wskazanej ścieżki, wymagany dodatkowy approve od użytkownika ze specifycznej grupy/roli. Przykładowo:

• Dla ścieżki src/data-processing/* wymagany dodatkowy approve z grupy Performance specialist
• Dla ścieżki src/core-engine/* wymagany dodatkowy approve z grupy Senior developer
• Dla ścieżki src/ssl-library/* wymagany dodatkowy approve z grupy Security specialist

Branch policy - all comments resolved

Branch policy - linked work item

Przykładowy PR z branch policy

Zaczynamy nowy feature

1. Tworzymy i pushujemy nowego feature brancha.
2. Tworzymy nowy draft pull request, który będzie na bieżąco w trakcie pracy raportował nam stan z CI.
3. Pracujemy tworząc i pushując małe commity często. Akceptowalne WIP/TEMP commity i push --force-with-lease. Często też robimy git fetch && git rebase origin/master.
4. Porządkujemy historię, układamy commity w odpowiedniej kolejności, łączymy gdy trzeba, usuwamy niepotrzebne.
5. Opublikowujemy drafta jako czekającego pull requesta i informujemy zespół.

Opublikowujemy pull requesta

1. Upewniamy się, że description PR wprowadza reviuującego w temat
2. Upewniamy się, że nie ma konfliktów lub czerwonych buildów
3. Po raz ostatni przeglądamy diffa wszystkich plików
4. Po raz ostatni przeglądamy listę commitów, które udostępniamy
5. Jeżeli serwer to umożliwia, ustawiamy automat do zamknięcia PR

Zaczynamy code review

1. Nie tykamy, jeżeli są konflikty lub buildy świecą się na czerwono
2. Zapoznajemy się z descriptionem pull requesta
3. Rzut oka na listę commitów
4. Przeglądanie kodu
5. W niektórych serwerach, komentarze mogą wskazywać zaznaczony tekst
6. Jeżeli serwer to umożliwia, gdy wystawiający PR dostosuje się do uwag, my przeglądamy tylko nowo wypchnięte zmiany
7. Wystawiający PR na zakończenie pilnuje usunięcia brancha

Niniejsza prezentacja w formie PDF

Prezentacja, na którą właśnie patrzysz, istnieje równiez w formie pdfa, niestety za pośrednictwem platformy webinarowej nie ma jak jej wysłać.

Jeżeli chciałbyś/chciałabyś ją sobie zachować, napisz do nas na kontakt@GitWarsztaty.pl i ją podeślemy.