Automating code reviews

Transcript

1. X Никита Соболев github.com/sobolevn 1
2. !2
3. >_ X Как не делать код ревью 3
4. Нет Да Как критиковать? • Критиковать код • Предлагать решение • Критиковать людей • Высказывать претензии 4
5. Нет Да Как критиковать? • Критиковать код • Предлагать решение • Критиковать людей • Высказывать претензии 5
6. Нет Да Как критиковать? • Критиковать код • Предлагать решение • Критиковать людей • Высказывать претензии 6
7. Нет Да Как критиковать? • Критиковать код • Предлагать решение • Критиковать людей • Высказывать претензии 7
8. Нет Да Как критиковать? • Критиковать код • Предлагать решение • Критиковать людей • Высказывать претензии 8
9. Нет Да Как критиковать? • Критиковать код • Предлагать решения • Критиковать людей • Высказывать претензии 9
10. Checklist: 10
11. Checklist: > Удостоверьтесь, что вы строите инклюзивную культуру 10
12. Checklist: > Удостоверьтесь, что вы строите инклюзивную культуру > Слушайте мнения всех людей 10
13. Checklist: > Удостоверьтесь, что вы строите инклюзивную культуру > Слушайте мнения всех людей > Мотивируйте свою команду 10
14. Checklist: > Удостоверьтесь, что вы строите инклюзивную культуру > Слушайте мнения всех людей > Мотивируйте свою команду > Учитесь коммуникации 10
15. Checklist: > Удостоверьтесь, что вы строите инклюзивную культуру > Слушайте мнения всех людей > Мотивируйте свою команду > Учитесь коммуникации > Be empathetic! 10
16. >_ X Стоп! 11
17. Если вы делаете код-ревью, то вы уже проиграли! 12
19. Любая привычка является "деланием", для функционирования деланию необходимы все его составные части 14
20. Деланье код-ревью 15
21. Неделанье код-ревью 16
22. • Гармония • Созерцание Деланье Неделанье • Рутина • Сложный процесс 17
23. • Гармония • Созерцание Деланье Неделанье • Рутина • Сложный процесс 18
24. • Гармония • Созерцание Деланье Неделанье • Рутина • Сложный процесс 19
25. • Гармония • Созерцание Деланье Неделанье • Рутина • Сложный процесс 20
26. • Гармония • Созерцание Деланье Неделанье • Рутина • Сложный процесс 21
27. Код-ревью за 15 минут или меньше 22
29. github.com/wemake- services/wemake- python-styleguide 24
30. >_ X Почему все плохо? 25
32. Цели код ревью 27
33. Цели код ревью > Контроль выполнения 27
34. Цели код ревью > Контроль выполнения > Контроль качества кода 27
35. Цели код ревью > Контроль выполнения > Контроль качества кода > Неповторение грабель 27
36. Цели код ревью > Контроль выполнения > Контроль качества кода > Неповторение грабель > Ревью архитектуры 27
37. Цели код ревью > Контроль выполнения > Контроль качества кода > Неповторение грабель > Ревью архитектуры > Ознакомление и обучение 27
38. Цели код ревью > Контроль качества кода > Контроль выполнения > Неповторение грабель > Ревью архитектуры > Ознакомление и обучение 28
39. Правильная цель 29
40. Код-ревью нужно за контролем корректности автоматики Правильная цель 30
41. >_ X Ознакомление и обучение 31
42. Проблемы обучения через код-ревью 32
43. Проблемы обучения через код-ревью > Неважные задачи 32
44. Проблемы обучения через код-ревью > Неважные задачи > Изолированная работа 32
45. Проблемы обучения через код-ревью > Неважные задачи > Изолированная работа > Всем пофиг 32
46. Проблемы обучения через код-ревью > Неважные задачи > Изолированная работа > Всем пофиг > Долго 32
47. Решение: маленькие задачи, онбординг и документация 33
48. Маленькие задачи – быстрая обратная связь 34
50. Онбординг: посмотрим на Open-Source 36
51. Checklist: 37
52. Checklist: > CONTRIBUTING.md 37
53. Checklist: > CONTRIBUTING.md > Developer Docs 37
54. Checklist: > CONTRIBUTING.md > Developer Docs > Architecture Decision Records 37
55. Checklist: > CONTRIBUTING.md > Developer Docs > Architecture Decision Records > Wiki 37
56. Checklist: > CONTRIBUTING.md > Developer Docs > Architecture Decision Records > Wiki > Issue and PR database 37
57. Checklist: > CONTRIBUTING.md > Developer Docs > Architecture Decision Records > Wiki > Issue and PR database > Human-readable tests 37
58. Задача: утром разработчик – вечером код 38
59. Для вдохновления
60. Для вдохновления > Gatsby.js
61. Для вдохновления > Gatsby.js > dev.to
62. Для вдохновления > Gatsby.js > dev.to > wemake-python-styleguide
63. >_ X Ревью архитектуры 40
64. Проблемы ревью архитектуры пост-фактум 41
65. Проблемы ревью архитектуры пост-фактум > Долго ждать 41
66. Проблемы ревью архитектуры пост-фактум > Долго ждать > Долго и дорого переделывать 41
67. Проблемы ревью архитектуры пост-фактум > Долго ждать > Долго и дорого переделывать > Участвует один человек 41
68. Решение: прототипирование и дизайн ревью до 42
69. И автоматизация! 43
70. Слои 44
71. Слои 45
72. [importlinter] root_package = django_project include_external_packages = True [importlinter:contract:layers] name = Layered architecture of our linter type = layers containers = django_project layers = urls views forms models logic 46
73. [importlinter] root_package = django_project include_external_packages = True [importlinter:contract:layers] name = Layered architecture of our linter type = layers containers = django_project layers = urls views forms models logic 47
74. [importlinter] root_package = django_project include_external_packages = True [importlinter:contract:layers] name = Layered architecture of our linter type = layers containers = django_project layers = urls views forms models logic 48
75. [importlinter] root_package = django_project include_external_packages = True [importlinter:contract:layers] name = Layered architecture of our linter type = layers containers = django_project layers = urls views forms models logic 49
76. [importlinter] root_package = django_project include_external_packages = True [importlinter:contract:layers] name = Layered architecture of our linter type = layers containers = django_project layers = urls views forms models logic 50
77. Независимость 51
78. [importlinter:contract:violation-independence] name = Independence contract for violations type = independence modules = django_project.billing_app django_project.auth_app django_project.orders_app django_project.statistics_app 52
79. [importlinter:contract:violation-independence] name = Independence contract for violations type = independence modules = django_project.billing_app django_project.auth_app django_project.orders_app django_project.statistics_app 53
80. [importlinter:contract:violation-independence] name = Independence contract for violations type = independence modules = django_project.billing_app django_project.auth_app django_project.orders_app django_project.statistics_app 54
81. Непротекающие абстракции 55
82. [importlinter:contract:api-restrictions] name = Forbids to import anything from dependencies type = forbidden source_modules = django_project.logic forbidden_modules = # Important direct and indirect dependencies: django rest_framework 56
83. [importlinter:contract:api-restrictions] name = Forbids to import anything from dependencies type = forbidden source_modules = django_project.logic forbidden_modules = # Important direct and indirect dependencies: django rest_framework 57
84. [importlinter:contract:api-restrictions] name = Forbids to import anything from dependencies type = forbidden source_modules = django_project.logic forbidden_modules = # Important direct and indirect dependencies: django rest_framework 58
85. [importlinter:contract:api-restrictions] name = Forbids to import anything from dependencies type = forbidden source_modules = django_project.logic forbidden_modules = # Important direct and indirect dependencies: django rest_framework 59
86. Можно писать свои контракты
87. >_ X Неповторение грабель 61
88. Проблемы повторения грабель 62
89. Проблемы повторения грабель > Вам больно 62
90. Решение: документация, регрессионное тестирование 63
91. sobolevn.me/2019/02/engineering-guide- to-user-stories !64
92. pypi.org/project/pytest-bdd !65
93. def test_regression71(): """ Testing that local variables are counted correctly. Regression test for #74. See: wemake-services/wemake-python-styleguide/issues/74 Regression test for #247 See: wemake-services/wemake-python-styleguide/issues/247 """ assert ... 66
94. И автоматизация! 67
95. !68
96. !69
97. !70
98. Упади, все плохо import { danger, fail } from 'danger' if (danger.github.pr.base.ref !== 'master') { fail('We only accept PRs to `master` branch.') } if (!danger.github.pr.rebaseable) { fail('Looks like your PR cannot be merged, please fix it: reopen or rebase.') } 71
99. Не падай, но скажи import { danger, warn } from 'danger' if (danger.github.pr.body.length < 50) { warn('Please provide a summary, at least 50 chars') } if (!danger.github.pr.body.match(/closes #\d+/i)) { warn('MR does not close any issues. Should close one') } 72
100. !73
101. Все подсветить 74
102. bellybutton 75
103. deprecated_fn() 76
104. !77
105. !78
106. !79
107. !80
108. !80
109. DeprecatedFnCall: description: Please use `new_fn` instead of `deprecated_fn` expr: //Call[func/Name/@id='deprecated_fn'] example: "deprecated_fn(*values)" instead: "new_fn(values)" 81
110. DeprecatedFnCall: description: Please use `new_fn` instead of `deprecated_fn` expr: //Call[func/Name/@id='deprecated_fn'] example: "deprecated_fn(*values)" instead: "new_fn(values)" 82
111. DeprecatedFnCall: description: Please use `new_fn` instead of `deprecated_fn` expr: //Call[func/Name/@id='deprecated_fn'] example: "deprecated_fn(*values)" instead: "new_fn(values)" 83
114. !85
115. >_ X Контроль качества 86
116. >_ X Контроль качества 87
117. >_ X Контроль качества 87 Том Круз смеется над теми
118. >_ X Контроль качества 87 Том Круз смеется над теми Кто думает, что может проверять качество кода глазами
119. Проблемы контроля качества 88
120. Проблемы контроля качества > Люди не умеют писать код 88
121. Проблемы контроля качества > Люди не умеют писать код > Сложность накапливается незаметно 88
122. Проблемы контроля качества > Люди не умеют писать код > Сложность накапливается незаметно > Метрики скрыты 88
123. Проблемы контроля качества > Люди не умеют писать код > Сложность накапливается незаметно > Метрики скрыты > Исправлять долго 88
124. Решение: самый жесткий линтер из доступных 89
125. Иногда нужно будет писать свой! 90
126. !91
128. Архитектура Код • Не знаем куда писать • Рискуем создать месиво • Сразу не знаем • Overengineering 93
129. Architecture on Demand sobolevn.me/2019/10/complexity-waterfall 94
132. Процесс: 97
133. Процесс: > Пишем простые блоки кода 97
134. Процесс: > Пишем простые блоки кода > В какой-то момент сложность переполняется 97
135. Процесс: > Пишем простые блоки кода > В какой-то момент сложность переполняется > Рефакторим 97
136. Автоматизируем все проверки github.com/wemake-services/wemake- python-styleguide 98
137. >_ X Контроль выполнения 99
138. Проблемы контроля выполнения 100
139. Проблемы контроля выполнения > Я не умею запускать код в мозгу 100
140. Проблемы контроля выполнения > Я не умею запускать код в мозгу > Я не знаю, что нужно клиенту 100
141. Проблемы контроля выполнения > Я не умею запускать код в мозгу > Я не знаю, что нужно клиенту > Не все ньюансы понятны из кода 100
142. Решение: BDD и Review Apps 101
143. sobolevn.me/2019/02/engineering-guide- to-user-stories !102
144. pypi.org/project/pytest-bdd !103
145. !104
146. !104
147. !105
148. !106
149. !107
150. >_ X Повторим 108
151. Цели код ревью 109
152. Цели код ревью > Контроль выполнения 109
153. Цели код ревью > Контроль выполнения > Контроль качества кода 109
154. Цели код ревью > Контроль выполнения > Контроль качества кода > Неповторение грабель 109
155. Цели код ревью > Контроль выполнения > Контроль качества кода > Неповторение грабель > Ревью архитектуры 109
156. Цели код ревью > Контроль выполнения > Контроль качества кода > Неповторение грабель > Ревью архитектуры > Ознакомление и обучение 109
157. Цели код ревью > Контроль качества кода > Контроль выполнения > Неповторение грабель > Ревью архитектуры > Ознакомление и обучение 110
158. Код-ревью нужно за контролем корректности автоматики Правильная цель 111
159. Инструменты код ревью 112
160. Инструменты код ревью > Маленькие задачи 112
161. Инструменты код ревью > Маленькие задачи > Review Apps 112
162. Инструменты код ревью > Маленькие задачи > Review Apps > Суровый статический анализ 112
163. Инструменты код ревью > Маленькие задачи > Review Apps > Суровый статический анализ > Строгие архитектурные правила 112
164. Инструменты код ревью > Маленькие задачи > Review Apps > Суровый статический анализ > Строгие архитектурные правила > Глаза человека 112
165. >_ X Нужно ли вообще смотреть в код? 113
166. Да! За контролем корректности автоматики 114
167. И кое-что еще: 115
168. И кое-что еще: > Корректные имена 115
169. И кое-что еще: > Корректные имена > Проверки гипотез 115
170. И кое-что еще: > Корректные имена > Проверки гипотез > Общая адекватность 115
171. Автоматизируйте ваш код-ревью 116
172. Код-ревью за 15 минут или меньше 117
173. Превратите деланье код-ревью в неделанье! 118
174. Что осталось за кадром? 119
175. Что осталось за кадром? > Обучение ревью 119
176. Что осталось за кадром? > Обучение ревью > Двойной ревью: CODEOWNERS vs случайный человек 119
177. Что осталось за кадром? > Обучение ревью > Двойной ревью: CODEOWNERS vs случайный человек > Ревью ревью 119
178. Что осталось за кадром? > Обучение ревью > Двойной ревью: CODEOWNERS vs случайный человек > Ревью ревью > Мотивация 119
179. Что осталось за кадром? > Обучение ревью > Двойной ревью: CODEOWNERS vs случайный человек > Ревью ревью > Мотивация > Аудиты 119
180. Что осталось за кадром? > Обучение ревью > Двойной ревью: CODEOWNERS vs случайный человек > Ревью ревью > Мотивация > Аудиты > Создание маленьких задач 119
181. t.me/ opensource_findings !120
182. Вопросы? github.com/sobolevn sobolevn.me