Tweet
Tweet

Slide 1

Slide 1 text

De edele kunst van het pull request Karl van Heijster www.karlvanheijster.com

Slide 2

Slide 2 text

Hoe ga je om met complexe pull requests?

Slide 3

Slide 3 text

Een ondoordachte approve leidt tot bugs in de code

Slide 4

Slide 4 text

Code reviews moeten ervoor zorgen dat de codebase continu verbetert

Slide 5

Slide 5 text

Maar code reviewen is moeizaam

Slide 6

Slide 6 text

Hoe kunnen we deze situatie verbeteren?

Slide 7

Slide 7 text

Hou bij het aanmaken van een PR rekening met de lezer

Slide 8

Slide 8 text

Hoe lees je een PR?

Slide 9

Slide 9 text

16 var linksAndErrors = manifest 17 .GetItemResources() 18 .Where(HasLomMetadata) 19 .Select(ir => (ct: ir.metadata.OfType(), href: ir.href)) 20 .SelectMany(tuple => GetMetadataLinks( 21 tuple.ct, tuple.href.Split('/').Last(), metadataContext)) 22 .ToList(); 23 24 var (links, errors) = SplitResults(linksAndErrors); 25 return metadataContext with { MetadataLinks = links, MetadataErrors = errors }; Is deze wijziging oké?

Slide 10

Slide 10 text

16 var linksAndErrors = manifest 17 .GetItemResources() 18 .Where(HasLomMetadata) 19 .Select(ir => (ct: ir.metadata.OfType(), href: ir.href)) 20 .SelectMany(tuple => GetMetadataLinks( 21 tuple.ct, tuple.href.Split('/').Last(), metadataContext)) 22 .ToList(); 23 24 var (links, errors) = SplitResults(linksAndErrors); 25 return metadataContext with { MetadataLinks = links, MetadataErrors = errors }; Kun je dat beoordelen op basis van de code alleen?

Slide 11

Slide 11 text

Een PR moet drie vragen beantwoorden

Slide 12

Slide 12 text

1. Waarom deze wijziging? 2. Wat doet de code concreet? 3. Hoe doet deze dat?

Slide 13

Slide 13 text

Bied de lezer alle informatie om de wijziging te kunnen beoordelen

Slide 14

Slide 14 text

Elke vraag heeft zijn eigen plek in het PR

Slide 15

Slide 15 text

1. Titel en beschrijving 2. Tests 3. Code

Slide 16

Slide 16 text

1. Titel en beschrijving 2. Tests 3. Code

Slide 17

Slide 17 text

De titel vertelt wat er wijzigt in de gebiedende wijs

Slide 18

Slide 18 text

De beschrijving vult details in over het probleem- en oplossingsdomein

Slide 19

Slide 19 text

/.azuredevops/pull_request_template.md Optioneel: gebruik een PR-template

Slide 20

Slide 20 text

<< Description of the changes in this PR. Make sure to mention the (design) choices you made and why. >> # Checklist - [ ] The title of the PR is as follows: . - [ ] The description explains *why* this PR was made, what the advantages of this change are. - [ ] The descriptions explains *what choices* were made *and why* the current implementation was chosen. - [ ] Unit tests validate this change.

Slide 21

Slide 21 text

1. Titel en beschrijving 2. Tests 3. Code

Slide 22

Slide 22 text

Tests documenteren het gedrag van een systeem

Slide 23

Slide 23 text

Schrijf tests op een gepast abstractieniveau

Slide 24

Slide 24 text

Tests beschrijven wat een systeem doet, niet hoe het dat doet

Slide 25

Slide 25 text

Vat je test samen in de titel

Slide 26

Slide 26 text

Benadruk het belangrijke

Slide 27

Slide 27 text

1. Titel en beschrijving 2. Tests 3. Code

Slide 28

Slide 28 text

Er bestaan legio boeken over het schrijven van goede code

Slide 29

Slide 29 text

Beschouw een PR als een informatieve tekst

Slide 30

Slide 30 text

Maak het makkelijk om PR’s te beoordelen

Slide 31

Slide 31 text

Zo voorkom je bugs en technische schuld

Slide 32

Slide 32 text

De edele kunst van het pull request Karl van Heijster www.karlvanheijster.com