Tips til forbedring (versjon 2)

(I'm sorry this issue is written in Norwegian, but it's generated by a script meant for the navikt organisation. Use Google Translate if you want to know what is written)

Dette er en autogenerert issue, laget av et skript som går gjennom alle NAV sine kodebaser på Github og gjør diverse sjekker. Her er en liste over ting som kan endres for at kodebasen skal bli enda bedre!

Kodebasen mangler en CODEOWNERS-fil

Dette er en fil som skal ligge i rotkatalogen, og angir hvilket team som eier kodebasen, på et maskinlesbart format. Den enkleste varianten, som vil holde for de fleste, er å ha en CODEOWNERS-fil som ser slik ut: (Merk at det skal være asterisk/stjerne foran navn på teamet!)

* @nais/teamnavn

Gyldige teamnavn på Github er:

@nais/aura (aura)
@nais/bots (bots)

Her er en oppdatert liste over team i Github. Mangler teamet deres i lista? Ta kontakt med noen i Core-teamet på Github, så kan de opprette et team til dere.

Hvis det trengs spesielle tilpasninger, så ligger det mer dokumentasjon om CODEOWNERS-filformatet her:

https://help.github.com/articles/about-codeowners/

Forslag: Bruk rebasing av pull requests, for å unngå merge commits i git-loggen

Ikke alle er enig i at merge commits skaper støy i git-loggen. Så dette er valgfritt. Men for de som synes det, så er det mulig å slå på en funksjon slik at alle pull requests gjøres med rebase i stedet for merge - da får man en renere git-logg.

For å endre dette, går man inn på Settings-panelet til Github-repoet, under avsnittet "Merge button" og slår av "Allow merge commits".

Forslag: Beskyttelse av master-branchen

Hvem som helst (av utviklerne i NAV) kan pushe til master-branchen i denne kodebasen. Det kan åpne opp for sårbarheter. For det første vil det en gang i blant skje uhell - utviklere commiter og pusher til en branch for å lage pull request, men så pusher de ved et uhell rett til master-branchen. Det åpner også opp for at noen som har tatt kontroll over en utviklerkonto på Github (eller en utro tjener) kan pushe kode rett til master-branchen og deploye direkte til produksjon, på egen hånd. (Kall det gjerne spekulativt, men det er en mulighet.)

Hvis man ønsker å gjøre noe med det, kan man gå i Settings-panelet til Github-repoet, velge "Branches", og under "Protected branches" sette opp master som protected.

Det er vel opp til teamet å bestemme selv nøyaktig hva kriteriene for push/merge til master skal være, men her kan man for eksempel legge inn krav om grønt bygg før merge, og/eller man kan kreve at koden må ha vært godkjent i code review av minst x antall utviklere fra teamet som eier kodebasen. Man kan også konfigurere at kun medlemmer av teamet som eier koden skal kunne merge pull requests.

Spørsmål og svar

Jeg har meninger om disse rådene - kan jeg komme med tilbakemeldinger?

Skriv i vei, på Slack-kanalen #open-source.

Kodebasen vår er ikke open source, derfor er det ikke noe poeng

Selv om koden i dag ikke er åpen for innsyn, så ta høyde for at den kan komme til å bli det i fremtiden. Uansett så vil forbedringene være til hjelp, enten kodebasen er åpen eller ei!

Hvem har ansvaret for å fikse det her?

Det er i utgangspunktet den/de/teamet som eier kodebasen som må gå gjennom.

Haster det?

Ikke egentlig. Men det hadde vært fint om det ble gjort, innen et par uker kanskje?

Huff, dette ble litt for mye jobb. Vi har ikke tid! Kan dere gjøre det for oss?

Det er greit. Bare skriv "kan dere gjøre det for oss?" som svar på denne issuen. Så kan vi gå gjennom sakene senere med et annet skript, og gjøre endringene automatisk!

nais / naisd