들어가며
사실 필자는 몇 달전 원티드 프리온보딩에 참여했을 때 PR 그리고 코드 리뷰라는 것을 처음 해보고 받아보았다.
이전 회사에서는 PR을 올리기는 하지만 워낙 바쁘고 하다보니 개인이 PR을 올려도 스스로 마지하고 개발하는 경우가 대부분이였다.
PR의 내용에는 간단한 개발 내용이 들어갈 때도 있지만, 아무도 안봐서 그런지 그 마저도 적지 않는 경우가 많았다.
(사실 Commit 메시지로 대체하는 경우가 많았다.)
하지만 이번에 원티드 프리온보딩 벡엔드 챌린지에 참여하면서 팀원들과 PR을 주고 받으면서 개발을 하게 되었다. 이때한 PR을 통해 리뷰를 주고 받는 경험이 너무 좋았다. 누군가의 리뷰요청이 오면 신나는 마음으로 코드리뷰를 했다. 또 누군가가 나의 PR에 대해서 궁금한 점에 대해서 물어보거나 나와 다르게 생각하고 있던 것들에 대해서 서로 의견을 주고 받는 과정 속에서 생각의 폭이 넒어 지는 경험을 하게 되었다.
관련해서 느낌점을 기록하고 싶어 포스팅한다.
(실제 PR을 보내고 Merge 목록들이다. 모든 부분에 대해서 리뷰를 받은 건 아니지만, 이렇게 보니 굉장히 뿌듯했다 😚 )
✅ 나에게 당연한 것이 상대방에게는 당연하지 않을 수 있다.
그렇기 때문에 최대한 상대방이 아무것도 모른다고 가정하여 설명하는 것이 좋다.
너도 알고 나도 알고 다 아는 데 굳이 그렇게 까지 해야해? 라고 말할 수 있지만, 이 작업은 매우 중요하다고 생각한다. 저마다 살아온 배경, 지식, 컨텍스트가 다르다. 내가 생각한 A 가 상대방에게는 B 라고 오해할 수 있는 상황이 생길 수 있다.
또한 글을 쓸 때는 가급적 지시대명사는 사용하지 말고, 누가 봐도 해당 지시대명사가 무엇을 가르키고 있는 지를 명확하게 알 때(혹은 알 수 있도록 글 작성) 사용해야한다.
✅ 남의 생각(클린코드, 기타 책의 코드)이 아닌. 그냥 너의 생각이 중요해!
실제 팀원과 리뷰를 한 코드를 가져왔다.
먼저 간단하게 코드에 대해서 설명하면 특정 음식점의 리뷰 유무(getRestaurantReviewStat.isEmpty()) 에 따라 리뷰 상태를 업데이트하는 코드이다. 리뷰가 없으면 그냥 저장하고, 리뷰가 있으면 업데이트 하고 따로 반환하는 값은 없다.
기존에는 if-else 로 되었던 구문이였는 데 리뷰할 때 early return 할 수 있도록 리뷰한 사례이다.
사실 필자는 early return 이라고 하는 것은 알고 있어서 코드에 최대한 적용하려고 노력했다. 그래서 if-else 로 된 로직을 early return 을 하도록 리뷰를 했다.
그 이후 팀원에게 왜 그렇게 하는 지 이유를 듣고 싶다고 리뷰가 왔을 때 사실 살짝 당황했다. 왜냐하면 필자 스스로가 당당하게 왜 사용하고 있는 지에 대한 이유를 설명할 수 없었기 때문이다.
물론 잠시 왜 사용하는 지에 대해서 스스로 납득할 수 있을정도로 생각하고 리뷰를 달긴했지만, 남의 코드 소위 말하는 클린코드나 깨끗한 코드를 짜는 방법만 외웠던 자신에게 반성하는 기회가 되었다.
✅ PR에 담긴 commit 은 리뷰할 때 빠르게 리뷰하는 힌트가 된다.
정신없이 개발하다 보면 나도 모르게 PR 에 담긴 commit의 개수,범위 그리고 변경되거나 추가된 파일의 수 등을 고려하지 않고 개발한다. 그러다 보니 피드백으로 pr 에 담긴 코드의 수나 commit 에 대한 제대로 된 규칙이 없었다. 결국 PR 에 대해 코드 리뷰를 하려고 해도 어떤 기능을 만들었고, 어느 부분을 봐야하는 지 시간을 많이 투자할 수 밖에 없었다.
그래서 PR에 담긴 commit 의 단위를 어떻게 주면 좋을 지에 대해서 팀원끼리 많이 이야기를 나누었다. 여러 의견이 나왔지만 하나의 commit 안에는 하나의 기능 내지는 변경점을 메시지로 나누고, commit 메시지 컨벤션을 정하여 쉽게 리뷰를 할 수 있도록 개선하였다.
또 PR을 올릴 때 눈에 더 잘 띄게 만들기 위해 PR 종류에 따라 이모지를 정하여 사용하기로 했다.
우리 팀원끼리 정한 규칙이다
✅ 보기 좋은 리뷰가 리뷰하기도 좋다
PR 은 내가 어떤 기능 혹은 코드를 변경했는 데 이 부분에 대해서 어떠한 지 모두에게 리뷰를 받는 과정이라고 생각한다.
만약 다른 리뷰어들이 어떤 기능 혹은 코드를 변경했는 지 한참 찾아야한다면 어떨까? 이는 상대방을 고려하지 않는 PR 이라고 생각한다.
이렇게 코드를 읽지 않더라도 어떤 내용을 개발했고, 어떤 로직으로 이루어졌는 지 그리고 관련해서 어떤 commit 을 봐야하는 지 까지 기록해 놓는다면 리뷰어는 리뷰하기가 매우 수월할 것이다.
실제로 한 우리 팀원의 PR 을 가져와봤다. 아래 PR을 보면 어떤 작업을 했는 지, 그리고 특히 유의해야할 부분이 어디인지 까지 설명하고 있어서 리뷰어 입장에서는 강약을 조절하면서 리뷰를 할 수 있다.
팀원의 리뷰를 토대로 PR을 작성해보았다.
깨알 지식
팀원 분 중 한분이 PR 템플릿 기능이라는 것을 소개해주셨다. 반복되는 PR 내용을 템플릿화 하여 어느정도 정해진 양식대로 PR을 작성할 수 있도록 도와주는 기능인데 매우 편리하다.
