Zautomatyzuj najbardziej uciążliwe elementy code review

Dowiedz się, jak zautomatyzować najbardziej uciążliwe elementy code review, oszczędzając czas oraz energię.

Tworzenie oprogramowania to złożone zadanie. Zdarza się, że program tworzy jedna osoba, jednak najczęściej nad oprogramowaniem pracują całe zespoły. Zespoły te mogą się składać z kilku lub kilkuset ludzi, a praca w tej samej bazie kodu z tak dużą ilością współpracowników stawia przed nami sporo wyzwań. Takie narzędzia jak git pomagają w równoległej pracy dzięki możliwości wersjonowania naszego kodu (widzimy, jakie zmiany zaszły) oraz widoczności zmian sugerowanych przez team. Ale im więcej ludzi pracuje w tej samej bazie kodu, tym więcej sugestii. A przecież ktoś musi je wszystkie przejrzeć.

Co więcej, code review jest niezbędne dla zapewniania wysokiej jakości pisanego kodu.

Ma ono kilka aspektów, m.in. wizualne, czyli sprawdzenie zastosowania się do przyjętych standardów pisania kodu (formatowanie, wcięcia, nawiasy) oraz logiczne, czyli poprawność, używanie wzorców architektonicznych, wydajność itd. Aby zredukować nakład pracy, jaki wynika z code review, możemy część tego procesu zautomatyzować. Skupimy się tutaj zatem na zautomatyzowaniu przeglądu kodu, biorąc pod uwagę takie czynniki jak styl i statyczną analizę kodu.

Przykłady z tego artykułu oparłem na Kotlinie (w aplikacji Androida), ale będzie je można dopasować do innych projektów, na przykład w Swift, Dart itd. Praca w zespole oznacza wspólne podejmowanie decyzji. Mogą być one związane z przepływem pracy, architekturą oprogramowania albo stylem kodu.

Niektóre z nich można wprowadzić do naszego workflow na stałe, np. do naszego IDE przez .editorconfig dla formatowania lub do pliku ze współdzielonym stylem kodu, który można zaimportować do IDE. Wielu decyzji nie będzie można udokumentować i wprowadzić w ten sposób i potrzeba innego miejsca, gdzie zostaną spisane. Pozwoli to obecnym i nowym członkom zespołu na zrozumienie podjętych decyzji i dopasowanie się do nich.

Spisywanie decyzji to jedna rzecz. Działanie w zgodzie z nimi to druga. Wymaga to jednomyślności zespołu. Każdy musi być ich świadomy i działać w zgodzie z nimi. Łatwo jest również o wielu rzeczach zapomnieć, ale code review zazwyczaj o nich przypomina.

Wraz z powiększaniem się zespołu i rosnącą liczbą pull requestów, czas, który spędzamy na code review również się zwiększa. Widziałem sporo pull requestów, w których większość komentarzy dotyczyła stylu lub innych rzeczy związanych z podjętymi wcześniej decyzjami, ponieważ ktoś o nich najwyraźniej zapomniał.

Zabiera to zdecydowanie za dużo czasu: czytanie kodu, skupianie się na stylu i wyłapywanie problemów, które trzeba potem zaadresować. Często problemy się duplikują, a to sprawia, że mamy zduplikowane komentarze.

Zobaczmy, jak to zautomatyzować.

Oto narzędzia, których użyjemy:

DangerJS to podstawowe narzędzie, którego użyjemy. Będziemy go uruchamiać dla każdego pull request i wykonywać kilka checków dla naszych zmian. Danger działa w trakcie procesu ciągłej integracji - daje to danemu zespołowi szansę na automatyzację najbardziej uciążliwych czynności związanych z code review.

Możemy napisać skrypt JavaScript, który będzie miał dostęp do pull request (oraz jego metadanych) i będzie mógł wykonywać checki.

Przykład: sprawdzamy, czy pull request modyfikuje plik changelog (CHANGELOG.YML):

// Add a CHANGELOG entry for app changes
const hasChangelog = danger.git.modified_files.includes("CHANGELOG.yml")

if (!hasChangelog) {
  fail("Please add a changelog entry for your changes. :crystall_ball:")
}

Jeśli autor nie doda zmian w changelog (inaczej: jeśli nie było żadnej modyfikacji tego pliku w pull request), to pojawi się komentarz.

Przykładowy komentarz zwracający uwagę na brakujące informacje w changelog

Redukuje to więc ilość czasu, jaką poświęcamy na code review. Można to rozszerzać, aby dopasować się do każdego workflow. Na przykład, upewniamy się, czy wersja oprogramowania zmienia się przed wydaniem albo ostrzegamy, jeżeli wykryte zostaną zmiany w krytycznych częściach systemu.

Danger JS ma pełny dostęp do commitów, jednakowo do katalogów w kodzie źródłowym oraz do wyniku procesu budowania. Pozwala to na wykonanie kilku kroków, zanim uruchomimy DangerJS w naszym potoku i wykorzystamy w jakiś sposób rezultaty.

Naszym celem jest automatyzacja tych elementów, które dotyczą problemów stylistycznych. Dodajmy do naszego potoku linter oraz statyczną analizę kodu.

Ze względu na to, że projekt jest w Kotlinie, to linterem będzie ktlint. Do statycznej analizy kodu użyjemy detekt. ktlint upewnia się, że nasz kod jest spójnie formatowany - mówiąc inaczej sprawdza, czy stosujemy się do tych samych reguł wcięcia, nazywania zmiennych, czy korzystamy z takich samych nawiasów itd.

Sprawia to, że różne części kodu zachowają ten sam styl, dzięki czemu łatwiej między nimi przechodzić. detekt utrzyma złożoność kodu na niskim poziomie. Zawiera zasady nt. korzystania ze współbieżności, optymalizacji wydajności oraz struktury kodu.

Aby dodać oba te narzędzia do potoku i umożliwić DangerJS korzystanie z ich raportów, musimy najpierw zezwolić na działanie raportów XML. Wtyczka DangerJS może odczytywać raporty sformatowane na wzór checklisty, co oznacza, że poza ktlint i detekt można też odczytać SwiftLint oraz raporty z innych linterów.

W tym przykładzie użyjemy GitHub Actions do wykonania pokotku CI, a ponieważ DangerJS będzie działał z pull requestami, to GitHub Actions dostarczy aktora GitHuba i token.

Workflow DangerJS w GitHub Actions wygląda następująco:

name: DangerJS

on: [pull_request]

jobs:
  build:

    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2
        with:
          fetch-depth: 1
      - name: Install Danger
        run: yarn add danger
      - name: Run Danger
        run: yarn danger ci
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Zainstaluje to Danger i uruchomi go w pull requestach. DangerJS wyszuka pliku konfiguracyjnego (Dangerfile) i go wykona. Należy to wykorzystać ze skryptem modyfikacyjnym changelog, który widać powyżej.

Następnym krokiem jest dodanie wtyczki lint report. Ponieważ mamy do czynienia z innym pakietem, to będziemy musieli go zainstalować.

name: DangerJS

on: [pull_request]

jobs:
  build:

    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2
        with:
          fetch-depth: 1
      - name: Install Danger
        run: yarn add danger [email protected] 
      - name: Run Danger
        run: yarn danger ci
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Teraz możemy dostosować naszą konfigurację DangerJS (Dangerfile) i dodać rapory z klint.

const reporter = require("danger-plugin-lint-report")

// Scan ktlint reports
schedule(reporter.scan({
    fileMask: "**/reports/ktlint/*.xml",
    reportSeverity: true,
    requireLineModification: true,
}))

Na koniec dodajemy ktlint do GitHub Action. Ponieważ raporty linterów nie są commitowane, a tym samym nie dodaje się ich do repozytorium, wyszukiwanie pliku **/reports/ktlint /*.xml nie wykaże żadnych raportów - i nie zostanie uruchomione.

Oto zaktualizowany plik workflow z dodanym krokiem do wykonania ktlint przed Danger:

name: DangerJS

on: [pull_request]

jobs:
  build:

    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2
        with:
          fetch-depth: 1
      - name: Install Danger
        run: yarn add danger [email protected] 
      - name: Run KtLint
        run: ./gradlew ktlintCheck
        continue-on-error: true
      - name: Run Danger
        run: yarn danger ci 
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Otwarcie danego pull request uruchomi teraz ktlint i DangerJS, który użyje zainstalowanej przez nas wtyczki do poszukiwania i analizowania raportów ktlint, oraz dodania komentarzy w review z opisem znalezionych problemów.

Pełna konfiguracja Androida z ktlint, detekt i Android Lint posiadałaby GitHub workflow który uruchomi każdy z nich:

name: DangerJS

on: [pull_request]

jobs:
  build:

    runs-on: ubuntu-latest

    steps:
      - uses: actions/checkout@v2
        with:
          fetch-depth: 1
      - name: Install Danger
        run: yarn add danger [email protected]
      - name: Run KtLint
        run: ./gradlew ktlintCheck
        continue-on-error: true
      - name: Run Detekt
        run: ./gradlew detekt
        continue-on-error: true
      - name: Run Android Lint
        run: ./gradlew lintDebug
        continue-on-error: true
      - name: Run Danger
        run: yarn danger ci
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

A tak to wygląda z Dangerfile:

import {danger, fail, message, warn, schedule} from 'danger'

const reporter = require("danger-plugin-lint-report")

// Scan ktlint reports
schedule(reporter.scan({
    fileMask: "**/reports/ktlint/*.xml",
    reportSeverity: true,
    requireLineModification: true,
}))

// Scan detekt reports
schedule(reporter.scan({
    fileMask: "**/reports/detekt.xml",
    reportSeverity: true,
    requireLineModification: true,
}))

// Scan Android Lint reports
schedule(reporter.scan({
    fileMask: "**/reports/lint-results-*.xml",
    reportSeverity: true,
    requireLineModification: true,
}))

Podsumowanie

W rezultacie członkowie teamu nie muszą zajmować się tak prozaicznymi rzeczami, jak styl kodu. Zaoszczędzi to na dłuższą metę sporo czasu i sprawi, że wszyscy będą się trzymać najlepszych praktyk.

Aby stale ulepszać taką konfigurację, reguły każdego z tych kroków można dostosować tak, aby idealnie pasowały do potrzeb zespołu.

Następnym krokiem byłoby napisanie niestandardowych reguł (dla każdej z nich), aby zautomatyzować nie tylko decyzje dotyczące stylu, ale także reguły dotyczące na przykład współbieżności (w Kotlinie mogą to być reguły dotyczące zasięgu Coroutine lub użycia kontekstu) lub inne, bardziej złożone tematy.

Oryginał tekstu w języku angielskim możesz przeczytać tutaj.