개발 조직에서 꼭 지켜야 할 코드 리뷰 원칙과 문화

728x90

IT 쪽에서 리뷰를 하는 작업은 내가 현재 알고 있는 내용들에 대해서 동료, 조직장, 기타 관련된 사람들에게 이슈들을 포함하여 발생한 이슈를 설명하는 것일 수도 있고 더 나은 것들에 대한 경험을 리뷰라는 항목으로 한다는 개념이다.

개발 조직에서 꼭 지켜야 할 코드 리뷰 원칙과 문화는 무엇이고 그에 정의해서 개발 업무에 국한되어 필자는 두가지 정도의 리뷰를 경험하고 리딩하였다.

이전 글에서는 코딩 규칙에 대해서 설펴 보고 어떤 규칙들을 만들어 가는지에 대해서 설명해 보았다.

개발업무와 관련된 문화(코딩규칙 정하기)

이전글에서는 전체적인 개발자가 개발을 하여 리포지토리에 푸시하고 머지하는 행위까지 전반적인 내용을 다루었다. 전체적인 개발 프로세스 정의 하기어느 개발 조직이든 비슷하게 개발완료

hbungshin.tistory.com

코드리뷰

일단 서비스를 개발하려는 로직과 서비스에 적용돼야 하는 내용을 개발자가 파악한 후 코드를 작성하는 행위를 하게 된다. AI 시대가 도래하면서 코드리뷰의 형태도 많이 틀려졌을 것이라고 생각한다. 그러나 본질은 바뀌지 않을 것 같다고 생각하지만 AI와 코딩에 대한 내용은 다음 기회에 한번 더 이야기하도록 하고 개발 문화로 범위를 좁혀서 일반적인 내용에 대해서 설명하고자 한다.

기본 원칙

	리뷰 받는 사람(Reviewee)	리뷰 해주는 사람(Reviewer)
기본 원칙	팀의 코드 컨벤션을 잘 지켰는지 확인한다. 코드 상의 논리적인 오류나 오탈자 점검한다. deprecated 된 기능을 사용하고 있는지 점검한다. 성능을 위한 최적화를 했는지 확인한다. 이전의 PR 에서 코멘트 받은 실수를 반복했는지 점검한다. 코멘트를 받을 것으로 예상되는 부분에 사전 설명을 추가한다. PR(MR) 을 올리는 코드 양은 최소화 한다.	로직에 대한 오류나 오탈자를 코멘트한다. 구현된 로직 중 개선할 수 있는 방법을 제안한다. 사용한 라이브러리가 신뢰할 수 있는 것인지 확인한다. 코드 상 이해가 안되는 부분에 대해 질문한다.

위의 7가지 원칙만 지켜도 코드리뷰 중 일어날 수많은 충돌을 사전에 방지한다. 위의 원칙은 복잡하지 않다. 다만, 지금 내가 가진 역량으로 내가 최선의 코드를 짜냈는지를 정확하게 드러내야만 수준 높은 코드리뷰가 이루어질 수 있다.

구체적인 액션 원칙 내용들

	리뷰 받는 사람(Reviewee)	리뷰 해주는 사람(Reviewer)
코드리뷰전 준비	코드 리뷰 전에 `개발노트`을 정리하자. 개발노트는 위키나 컨플런스에 짧게 정리하거나 개발 후 나의 로직 점검에 유용하다. 모두에게 어떤 방식으로 개발을 진행할지 구조에 대해 공유를 먼저 하고 코딩을 진행하자. 많은 시간을 들이지 않는 선(오버헤드가 되지 않는 선)에서 최대한 간단하게 정리한다. 개발 노트 을 PR에 첨부하자. 모든 작업에 개발 노트가 필요하지는 않다. 작업 분량이 크거나 구조적인 변경이 큰 경우에 자율적인 판단으로 작성하자.	코드의 논리적인 오류 혹은 스멜을 탐지해주는 것에 중점을 두자. 코드 리뷰 본연의 목적이라고 본다. 자동화되지 않았다면 컨벤션 지적도 중요하다고 본다. 컨벤션 자동화가 되었을 경우 자동화로 체크하지 못하는 네이밍 규칙 등에 대해서는 여전히 지적한다. 만일 클라이언트 자동화(정적도구 사용)를 하고 있는데도 불구하고 안된다면
기본 절차	수정 및 추가한 부분에 대한 사이드 이펙트가 우려되는 부분을 명시하자 `개발 노트` 하단에 정리하여 PR에 첨부	구조적인 문제를 거론할때는 신중하게 접근하자. 구조의 경우 뒤집는데 많은 코스트가 든다. 크리티컬한 문제가 있지 않다면, 전반적으로 담당자가 의도한 구조와 방향성을 신뢰해주자. 구조 변경이 시급하다면 PR이 아닌 따로 짧은 일정을 잡아서 으논의하자.
진행 절차	PR은 큰 덩어리가 아닌 작업 규모에 맞는 덩어리로 나눠서 올린다. 작업 A와 작업 B를 코딩했다면 작업 A에 대한 PR과 B에 대한 PR을 따로 작성한다.	개인 의견과 프로젝트의 방향성은 구분한다. 개인 의견과 프로젝트 전반적인 방향성이 다를 수 있음을 인정하고, 프로젝트 전체적인 방향에 집중하자. 개인의 의견에 옳고 그름은 없지만, 프로젝트의 방향성에 도움이 되느냐 되지 않느냐는 명확하다. 거대한 덩치의 프로젝트를 움직이려면 어느 정도의 개인 의견 희생이 필요하다. 이견이 있더라도 그 최종 결정은 팀 리더가 내린다는 상징으로써 PR "Merge 버튼" 은 팀 리드가 누르는 것을 원칙으로 한다. 응급시에는 팀원 누구나 누를수 있다.
리뷰어 지정	PR 의 reviewer 지정이 필수는 아니지만(강제로 2명 이상 설정이라든가 조직마다 정하면된다) 해당 PR 에 대해 누군가가 꼭 확인했으면 좋겠다싶을 때는 reviewer 로 지정한다. PR 내의 커밋들을 병합하고 싶지 않다면 "Don't squash" label 을 달아서 의도를 표현한다.(없으면 만들기) PR 의 리뷰를 받고 싶지만 merge 되면 안될때에는 "Don't merge" label 달아서 의도를 표현한다. (없으면 만들기)	PR 생성자가 요청한 reviewer 가 있으면, 해당 리뷰어가 리뷰를 할때까지 pr merge 하지 않는다. PR Merge 의 기본 전략은 "Squash & Merge" 이고, 개별 커밋을 남기는게 의미가 있을 때에는 "Rebase & Merge" 를 사용한다. "Squash & merge" 와 "Rebase & merge" 가 둘다 부적절/불가능한 상황일때에는 일반 "Merge" 를 사용한다.
완료 후 작업	PR 은 개인 folk 보다, 공용 repo 브랜치를 만들어서 올리되 담당자를 구분할 수 있도록 브랜치 이름을 만든다. (예, "아이디/flutter_login")	PR 이 merge 되면, 너무 많은 branch 가 쌓이지 않도록 "merge 버튼" 아래에 있는 "Delete branch" 버튼으로 브랜치를 지운다.

대부분의 내용들은 회사의 상황 , 개발자의 역량으로 변경이 충분히 가능하다고 생각한다. 리뷰어 필수를 1명을 지정한다던가 또는 개발한 내용의 정리하는 것들, 그리고 왜 이렇게 했는지에 대한 여부 등등
그러나 가장 중요한 내용은 회사 입장 혹은 조직, 팀의 입장에서 보면 아래 내용은 꼭 지키게끔, 지켜야 하는 것이라고 생각한다.

개인 의견과 프로젝트의 방향성은 구분한다.
개인 의견과 프로젝트 전반적인 방향성이 다를 수 있음을 인정하고,
프로젝트 전체적인 방향에 집중하자.개인의 의견에 옳고 그름은 없지만, 프로젝트의 방향성에 도움이 되느냐 되지 않느냐는 명확하다.

이 내용이 그르치게 되면 팀웍과 조직의 방향성이 흐트러지게 되고 조직의 성과와 성장에 방해가 된다.

스타트업이든 어느정도 규모가 있는 회사든지 간에 회사의 방향성을 이해하고 그 이해된 신뢰를 바탕으로 코드를 작성하는 것이 가장 중요하다.

스타트업은 당장 우리의 아이디어, 메인 가치가 시장에 먹힐 것인가를 고민해야 하는데 구구절절한 절차를 가지고 한다는 것은 난센스가 될 것이고 어느 정도 고객을 확보하고 시스템의 안정성, 절차 그리고 누군가 새로 들어오는 멤버들에게 합리적으로 설명해야 하는 곳이라고 절차와 과정이 갖추어져 있어야 그 신규 직원도 신뢰를 가지고 움직 일 수 있을 것이기 때문이다.

서비스 리뷰

서비스 리뷰는 개발 하기전 혹은 개발을 완료된 후에 그 기획을 한 기획자, 운영자, 마케터들에게 현재 내가 구현되어 있는 로직이 무엇이고 예외상황에 대해서 리뷰를 하는 것이다.

이과정이 중요한 이유는 기획의 문서를 가지고 코드로 녹여내는 작업을 해야 하는데 100% 반영되기 어려운 예외 상황들이 항상 존재하기 때문이다.

또한 플랫폼이 다양하게 서비스를 하고 있는 회사일 경우 Android,iOS,PC, 웹 등 다양한 클라이언트 뷰를 가졌을 경우 서로의 로직을 상반되게 생각할 수도 있고 더군다나 기술의 특성상 적용의 한계 때문에 다르게 풀어가는 것들도 존재하기 때문이다.

절차

보통은 서비스 기능 개발 담당자가 메신저 혹은 슬랙 등 온라인으로 먼저 물어보게 된다. 그러나 이 부분의 단점은 2명의 대화자가 이야기를 해서 결론이 나긴 해도 최종 문서에 업데이트가 되지 않거나 말로의 서로 생각들이 불일치도 될 수 있다. 이 히스토리는 2명만 아는 것일 확률이 높기 때문이다. 그래서 이럴 때는 반드시 회의록 작성을 하거나 스크럼, 주간회의 때 꼭 기록하는 습관을 가지자.

QA 전 전체 프로세스 리뷰

대부분 기능들은 일정 기간 개발 기간을 거친 후 통합적인 QA 를 진행한다. 일전에 쓴 글에도 명시를 했지만 개발을 완료 뒤에 문서 혹은 구두로 아니면 개발자가 직접 예외 상황을 처리한 경우 개발 일정에 통합 리뷰 시간을 가져서 궁금한 사항들, 기획자의 의도가 맞는 것인지, QA 가 생각하는 내용과 문서 작성에 필요한 내용들에 대해서 전체 리뷰를 가지는 시간이 있어야 한다.

결론

개발 문화에 대해서 작성하는데 리뷰란 내용을 2가지를 나누어서 설명하였다. 이미 자기 조직에서 대부분 시행하고 있는 것들도 있을 것이다. 무수히 많은 요구사항이 쏟아지고 더군다나 지속되게 바뀌는 요구사항을 한 번에 정리하긴 어렵지만 적당한 이정표를 만들고 그 시간에 정리하는 시간을 가져야 코드의 지속성과 건전성이 향상되며 그리고 개발자의 신뢰도에 많은 영향을 줄 수 있다고 생각한다.