코드리뷰 어떻게 하고 계시나요?
코드리뷰 미니 워크숍
안녕하세요, iOS 개발하는 웅쓰입니다.
너무도 오랜만에 인사드리네요. 어디 갔던 건 아니고요. 개인적으로 이래저래 이런 글을 쓸만한 여력이 되지 않아...
네 다 핑계겠지요. 벌써 5월 말이라니, 23년 첫 글이라니,,, 반성으로 시작합니다 :(
오늘은 지난번에 저희 파트에서 했던 코드리뷰 미니 워크숍에 대한 기록입니다.
배경
1. 저희 파트는 작년 말에 코드리뷰 템플릿을 만들고 시간 피드백을 시작했었습니다. (+ 선플달기 운동(?)도 했었습니다. 잘 지켜졌는지는 개인차가 있았던 것 같지만...)
2. 사실 제가 올해 우리 파트의 코드리뷰를 위해 무언가 해보기로 했는데, 너무 막막해 뭘 하면 좋을지 직접 여쭤보고 싶었습니다.
3. 다들 어떻게 코드리뷰 하고 계시는지 궁금했고, 작년에 만든 코드리뷰 템플릿은 어떠신지, 시간 피드백은 어떻게 생각하시는지, 그래서 앞으로 무엇을 더 해보면 좋을지 궁금했습니다.
1. 사전 질의응답
그렇게 사전 질문을 몇 개 준비해 갔고 다들 손으로 열심히 적어주셨습니다.
1.1. 코드 리뷰 하는 과정(순서)을 적어주세요.
각자의 스타일을 엿볼 수 있었는데요, 개인적으로 다른 분들이 어떤 생각의 흐름으로 리뷰를 하시는지 배울 수 있어서 좋았고 듣는 것 자체로도 많이 배웠던 것 같습니다. 하여 날 것 그대로 옮겨봅니다.
설명 읽기 → 코드보기 → approve or 코멘트 → merge
1. 설명을 읽는다.
2. 변경된 class위치와 이름을 본다.
3. view model → domain → repository impl → data source → domain → usecase → view model 순으로 data 흐름을 보고
4. presentation에 어떻게 view를 구성했는지 본다.
모든 PR을 한 번씩 열어본다.
파일이 작은 것부터 시작
한 번에 볼 수 있는 pr은 그 자리에서 approve
그 외 pr들은 안스로 이동
코멘트 or approve
빠르게 할 수 있는 것 or 그렇지 않은 것 구분 → 모든 PR을 tab에 열어둔다.
pr 설명을 통해서 무엇을 하려는지 이해하기 → 안 읽히면 믿음의 머지
코드의 구성이 적절한지 (논리, API 추상화)
아키텍처상 코드의 위치가 맞는지 확인 (반복이나 클린 아키텍처 위치)
빠르게 확인할 수 없는 코드인 경우 IDE에서 PR을 확인하고 commit 단위로 확인. 필요시 빌드
PR의 크기 확인
디스크립션 (필수) → git diff (필수) → xcode 실행 (선택)
코멘트 설명 → branch checkout → 시뮬에서 기본 동작 테스트 (14 pro, se, ipad) → 데이터 flow를 따라 동작 구조 파악 → 코드 컨벤션 체크 → 개선점 체크 → 코멘트 & 머지
어떤 종류의 PR인지 파악 (디스크립션 보고)
기능을 잘 구현했는지 파악 (동작 + 기존 코드와 결이 맞는지 + 버그는 없는지 + 멀티 디바이스 대응)
사이드 이펙트가 발생할 수 있는지 체크
네이밍, 오타
브랜치 빌드 → 디스크립션 읽기 → 동작 이상 없는지 확인 → diff 읽으면서 코드 이해, 눈에 보이는 부분 리뷰 → 더 좋은 방법이 없나 고민
1.2. 코드 리뷰에서 중점적으로 보는 것을 중요한 순서대로 적어주세요.
여기도 그대로 옮겨봅니다. 각자 생각하는 좋은 코드의 기준들이 조금씩 드러났던 것 같습니다.
코드, 스펙
(1) data 흐름
(2) 각 요소들이 올바른 위치에 있는지
(3) 함수, 변수 이름
(4) 중복 확인
내가 알고 있는 히스토리와 같은가
추가한 설명과 구현이 일치하는가
논리적 오류가 있는가
기존 코드 컨벤션에 어긋나지 않는가
전체적 flow → 로직 → 컨벤션 → 네이밍이나 오탈자
다음에 봐도 쉽게 이해가능한지
기능의 이상이 없는지, 유지 보수에 좋은 코드인지, 기존 틀에 맞는지
1.3. 작년에 만든 코드리뷰 템플릿을 적용해 보니 어떠셨나요? 좋았던 점이나 개선점에 대해 피드백 부탁드립니다.
기존 코드리뷰 템플릿은 이렇게 생겼었습니다.
유용한 양식도 있지만 주로 불필요한 것들이 많다는 의견이 많았고 다음과 같이 수정되었습니다.
1.4. 이제 시작하고 있는 시간 피드백을 해보니 어떠셨나요? 좋았던 점이나 개선점에 대해 피드백 부탁드립니다.
*시간피드백이란?
리뷰하는데 얼마나 걸렸는지 피드백 주는 것
장점으로는 의식적으로 더 작은 PR을 만들게 되었다, 피드백을 더 짧게 주기 위해 더 집중해서 리뷰하게 되었다. 등이 있었고
단점으로는 댓글이 너무 많아서 중요한 댓글 확인이 어렵다, 정확한 시간 측정이 어렵고 순수 작업 내용뿐만 아니라 기존 코드에 대한 이해가 필요한 경우도 많다. 등이 있었습니다.
기타 의견이나 개선점으로는 구체적인 몇 분보다는 5min, 10-20m 같이 범위를 정하면 좋겠다, 작성자가 예측치를 적고 거기서 많이 벗어난 경우에만 피드백하면 좋겠다 등의 내용들이 나왔었습니다.
1.5. 매년 본인의 코드 리뷰는 발전하고 있다고 생각하시나요? 발전했다면 혹은 아니라면 그 이유는 무엇일까요?
발전하고 있다와 관련된 의견으로는
기존 코드의 숙련도 증가, 더 작아진 PR 단위, 좋은 리뷰를 받아서, 나름의 원칙이 생기고 있다. 등이 있었습니다.
다른 의견으로는
정답이 없으니 성장 여부는 모르겠다. 남이 판단해줘야 한다, 어떻게 평가할 수 있을지 잘 모르겠다. 등의 의견이 있었습니다.
2. 더 이야기해 보기
위 질의를 나누며 몇몇 이야기를 더 나눠볼 수 있었습니다.
2.1. 디스크립션은 왜 안 읽힐까?
깃헙 디스크립션이 너무 촘촘해서 잘 안 읽힌다는 의견부터, 이해를 위한 히스토리도 중요한데 이를 디스크립션에 옮겨 적는 게 힘들다는 의견도 있었습니다. 또 이를 받아들이는 히스토리에 관한 개인의 수준차가 있기 때문이라는 의견도 있었습니다.
2.2 리뷰를 잘하려면?
코드리뷰 하는 best practice를 찾아 리스트업 하자. (e.g. jinny)
리스트업 해서 순서가 정해진다면? 리뷰하는 순서가 중요할까?
면접 볼 때 코드리뷰 시뮬레이션을 시켜보면, API 추상화, 사이드 이펙트를 어떻게 확인하는지 등등이 궁금한데 본인만의 케이스를 100점으로 대답한 사람이 거의 없음.
결국 정답은 없고 본인만의 답을 찾아야 함.
2.3 네이밍에 대하여
의견 달기 조심스러움
특히 늦게 합류한 사람은 의견 달기 조심스러움
그렇게 생각할 수도 있구나 해서 잘 코멘트 안 다는 편
ㄴ 잘하는 사람들과 함께해서 그렇지 하고 있는 것일 수 있음 (정말 용납 안 되는 건 코멘트할 것임)
이 네이밍이 제대로 설명을 못하는 경우 코멘트
2.4 안 읽히는 코드는 어떻게?
어떤 게 안 읽히는가?
빌드 돌릴 거랑 안 돌릴 거 구분이 안 되어있을 때
이쪽저쪽 수정이 많을 때
-> 기존 코드의 컨벤션이 틀려도 리네이밍, 스페이싱도 건드리지 않는 게 원칙
-> 그렇지만 그러면 아무것도 건드리면 안 됨, 매번 따로 PR 날리기 부담
-> 적절한 선을 찾아야 함
.
.
.
급 결론, 이로써 새롭게 정의된 우리의 Ground Rule
코드뿐만 아니라 Request에 대한 피드백도 고려해서 적자.
생각보다 시간이 부족해 더 많은 이야기를 나누지 못해 아쉬웠으나, 그래도 서로의 방식과 생각을 공유할 수 있는 시간이어서 좋았고 각자 코드 리뷰에 대해 한번 생각해 볼 수 있는 시간이어서 좋았던 것 같습니다. (그랬었으면 하는 바람...ㅎ)
이 이야기를 나눈 뒤로 벌써 몇 개월의 시간이 흘렀습니다.
팀원도 더 늘었고 저희의 업무도 더 다양해졌습니다.
자연스럽게 코드리뷰도 더 어려워지고 새로운 장치에 필요성도 생겼던 것 같습니다.
그로 인해 pr-assistant, assignee review 등이 도입되었습니다.
이제는 다시 우리의 코드리뷰는 괜찮은가 점검해 볼 시간이 온 것 같습니다.
사실 당장 내일 다음 코드리뷰 세션을 하는데요, 이 글도 그 준비과정의 일환이기도 합니다.
(매번 이렇게 끝에 와서 벼락치기 느낌으로 일하는데 다음부턴 생각과 고민을 멈추고 빨리 행동할 수 있는 제가 되겠습니다...)
다음 후속 이야기를 들고 다시 찾아뵙겠습니다.
읽어주셔서 감사합니다.
