De edele kunst van het pull request

devNetNoord 2023

Transcript

1. De edele kunst van het pull request Karl van Heijster www.karlvanheijster.com
2. Hoe ga je om met complexe pull requests?
3. Een ondoordachte approve leidt tot bugs in de code
4. Code reviews moeten ervoor zorgen dat de codebase continu verbetert
5. Maar code reviewen is moeizaam
6. Hoe kunnen we deze situatie verbeteren?
7. Hou bij het aanmaken van een PR rekening met de lezer
8. Hoe lees je een PR?
9. 16 var linksAndErrors = manifest 17 .GetItemResources() 18 .Where(HasLomMetadata) 19 .Select(ir => (ct: ir.metadata.OfType<ClassficationType>(), 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é?
10. 16 var linksAndErrors = manifest 17 .GetItemResources() 18 .Where(HasLomMetadata) 19 .Select(ir => (ct: ir.metadata.OfType<ClassficationType>(), 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?
11. Een PR moet drie vragen beantwoorden
12. 1. Waarom deze wijziging? 2. Wat doet de code concreet? 3. Hoe doet deze dat?
13. Bied de lezer alle informatie om de wijziging te kunnen beoordelen
14. Elke vraag heeft zijn eigen plek in het PR
15. 1. Titel en beschrijving 2. Tests 3. Code
16. 1. Titel en beschrijving 2. Tests 3. Code
17. De titel vertelt wat er wijzigt in de gebiedende wijs
18. De beschrijving vult details in over het probleem- en oplossingsdomein
19. /.azuredevops/pull_request_template.md Optioneel: gebruik een PR-template
20. << 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: <FEATURE|BUG|REFACTOR> <Short description formulated as command>. - [ ] 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.
21. 1. Titel en beschrijving 2. Tests 3. Code
22. Tests documenteren het gedrag van een systeem
23. Schrijf tests op een gepast abstractieniveau
24. Tests beschrijven wat een systeem doet, niet hoe het dat doet
25. Vat je test samen in de titel
26. Benadruk het belangrijke
27. 1. Titel en beschrijving 2. Tests 3. Code
28. Er bestaan legio boeken over het schrijven van goede code
29. Beschouw een PR als een informatieve tekst
30. Maak het makkelijk om PR’s te beoordelen
31. Zo voorkom je bugs en technische schuld
32. De edele kunst van het pull request Karl van Heijster www.karlvanheijster.com