[회사생활] Code Review

Photo by Alvaro Reyes on Unsplash

이직을 한지 2주 ..

현 회사는 코드 리뷰 프로세스가 아직 정립되어 있지 않아서 이에 대한 도입을 제안해보았다. 코드리뷰는 에러방지와 리팩토링의 원활함과는 별개로 개발자를 더욱 성숙시키는 툴이라고 믿기 때문에 이에 대한 내용을 제안하게 된 것이다.

물론 나 역시 이 프로세스에 익숙해지는데 시간이 걸렸고 여전히 미성숙한 부분이 많다. 하지만 잘하기 위해 필요한 것이 연습이고 시작을 하지 않는다면 결코 잘한다는 단계에 도달할 수 없다.

바로 뛰어 들기 보다는 어떤 식으로 진행이 되는지를 나누고 이에 따라 조금씩 가능한 시간에 부담없이 조금씩 팀들이 함께 참여하기를 원한다.

아래는 이를 제안하며 작성했던 글이다.

코드리뷰, 왜 해야 할까?

코드리뷰는 프로덕트 QA로 확인할 수 없는 코드의 안전성과 컨벤션 그리고 더 나은 구조를 위한 최소한의 안전장치이다.

다른 사람의 눈을 빌어 실수를 줄일 수 있으며 누구나 이해하기 쉽고 더 생산적인 코드를 만들 수 있으며 이 과정을 통해 자연스럽게 팀내의 코드에 대한 컨벤션이나 싱크를 맞추는 것이 가능하다.

• 코드의 일관성 : 여러사람의 눈을 거친 코드는 한 사람의 개성이 아닌 한 팀에 맞는 일관성을 지니게 된다.
• 지식 나눔 : 코드 리뷰를 통해 자신이 알고 있는 지식을 나누거나 혹은 남의 지식을 요청할 수 있다.
• 새로운 방법 : 기존의 방법이 항상 옳지는 않다. 하지만 개인 혼자서 변경을 도모하는 것은 어렵다. 생각지 못했던 새로운 방법을 제시받거나 새로운 방법을 시도해볼 수 있는 장을 마련할 수 있다.
• 에러방지 : 미처 생각하지 못한 에러를 타인의 눈을 통해 발견할 수 있다.
• 성장: 리뷰를 하며 개발자들은 자신의 생각을 타인과 나누고 이를 소화하는 과정에서 더 우아한 방법을 배울 수 있다.

물론 코드리뷰가 좋은 점만 있는 것은 아니다. 이는 시간을 들여야하는 또 다른 태스크로 개발자의 리소스를 요구한다. 하지만 이 리뷰를 통해 다른사람과 의견을 공유하고 이를 받아들이고(혹은 이에 대한 자신의 생각을 정리하고 반론하며) 스스로 더 나은 개발자로 발전할 수 있는 툴이라는 점과 코드의 오류를 QA이전에 조금이라도 잡을 수 있다는 점에서 리소스 대비 더 많은 이익을 주는 것은 분명하다.

코드리뷰, 무엇을 리뷰할까?

- 전반적

1. 컨벤션 : 코드가 기존 컨벤션에 맞게 처리되었는지 확인하며 대부분의 경우 prettier나 eslint에 의존적이므로 이에 대한 에러가 없는지 확인한다.
2. 이름(변수명, 메서드 이름): 리뷰어는 컴퓨터가 아니므로 변수나 메서드 이름은 중요하다. 이는 로직을 더 쉽게 이해하고 해당 데이터가 수행하는 일과 관련해 지어야한다. 단순한 이름이나 너무 일반적인 이름일 경우 다음 코드를 보는 사람에게 잘못된 이해를 제공할 수도 있다. (짧은 이름보다는 길지만 묘사적인 이름이 좋다.)
3. 함수나 메서드가 처리하는 로직: 해당 함수의 로직이 수행하려는 바를 정확히 수행하는지.. 너무 많은 책임과 작업을 하고 있지 않은지 확인한다. 함수의 길이는 12줄, 최대 15줄을 넘지 않는것이 좋다.
4. 로직의 위치: 해당 로직이 해당 컴포넌트 뷰에 한정적인 것인지 비지니스 로직인지, 혹은 차후 앱전체에서 사용할 내용인지에 따라 동일한 파일 혹은 다른 파일(동일한 컴포넌트 혹은 다른 컴포넌트나 컨셉등)로 위치시킨다.
5. 하나의 pr은 하나의 fix나 feature를 가진다. 만약 이에 따른 리팩토링이 필요하다면 리팩토링과 fix브랜치를 따로 관리한다. refactoring 브랜치는 가능한 먼저 머지한다.

- Angular (혹은 바탕이 되는 프레임워크| 라이브러리)

1. 컨셉: 앵귤러의 경우 컴포넌트 디렉티브 파이프 서비스 등의 여러가지 컨셉을 제공한다. 이에 맞는 컨셉으로 지정되었는지 확인한다.
2. rxjs의 subscription 해제는 중요하다. 컴포넌트 해제될때 남아있는 subscription은 없는지 확인한다.
3. changeDetectionStrategy: angular의 zone.js는 좋은 선택이 아니다. 필요하지 않게 변경감지프로세스를 돌리는일이 많으므로 기본은 onPush가 좋다.
4. 라이프사이클: 해당로직이 적절한 라이프 사이클에서 사용되었는지 사용하지 않는데 추가된 인터페이스는 없는지 확인한다.

- Style

1. 스타일이 깨어지는 곳이 없는지 확인한다.(리뷰용 폴더를 하나 따로 만들어 해당 파일을 로컬에서 돌려보자.)
2. class 이름이 unique한지 확인한다.
3. 필요없이 들어간 property가 없는지 확인한다.
4. >, +등의 실렉터가 적정하게 쓰였는지 확인한다.

코드리뷰, 어떤 자세로 해야할까?

코드리뷰는 실수를 지적하는 프로세스가 아니다. 이는 더 좋은 방법을 제안하고 잘된 부분을 함께 나누는 프르세스이다.
하지만 더 나은 해결책을 제시하는 과정에서 상대의 기분을 상하게 할 수 있는 여지는 많다.

코드리뷰는 특정 목적을 위한 툴이라는 시각에서 이를 이용할때의 reviewer와 commiter로서 갖춰야하는 조건도 분명히 존재하는 것이다.

- Commiter

1. 리뷰어는 작업에 대한 지식을 가지고 있지 않다. 이에 대한 scope와 전반적인 배경에 대한 설명을 먼저 밝힌다.
   (지라이슈나 document로 이를 대신해도 된다.)
2. 하나의 pr은 하나의 scope나 feature 를 가진다. 리팩토링이 필요하다면 이를 따로 불리해서 먼저 refactoring pr을 요청하고 이를 기반으로 현재 작업에 대한 pr을 처리, 총 2개의 pr로 요청한다.
3. 하나의 이미지는 100개의 말보다 더 빠른 이해를 돕는다. 스타일이나 기능의 시각적 요소가 중요하다면 설명에 이미지를 추가한다.
4. 리뷰는 객관적인 행위이다. 하지만 코드를 만드는 일은 주관적이기 때문에 reviewer로서와 commiter로서의 시선을 미리 연습해보는 것은 좋은 일이다. 자신의 코드 리뷰를 요청하기 전에 자신이 미리 리뷰를 해보는 것은 좋은 방법이다.
5. 타인의 리뷰는 상처를 주려는 행위가 아니다. 그러니 겁 먹을 필요는 없다.
   타인의 제안은 겸손하게 받아되 자신의 이유가 있다면 이를 정확하게 밝히는 것이 좋다.
   만약 스스로 고민했던 부분이나 다른 제안을 가지고 있다면 이를 리뷰를 요청하기전에 미리 밝혀두면 한번의 pingpong을 막을 수 있다.
6. 무조건적인 수긍 역시 좋지 않다. 만약 이야기가 길어지는 이유가 프로덕트에 치명적이지 않은 요소라면(이를테면 네이밍이나 컨벤션 관련 요소)라면 해당요건에 대해서는 배포 이후 다시 논의를 요청하는 것도 좋은 방법이다.
7. 새로운 방법을 사용했거나 이에 대한 이해가 어려울 경우 codesandbox나 타사이트에 예시코드를 만드는 것도 방법이다.
   https://codesandbox.io/(추가: 코드샌드박스는 angular관련 에러가 많아 사용하지 않는 것이 좋은 것 같음)
   https://ng-run.com/
   https://stackblitz.com/angular/mxmankeexvm?file=src%2Fapp%2Fheroes%2Fheroes.component.html

- Reviewer

1. 리뷰는 남의 코드를 헐뜯는 행위가 아니다. 정중하게 다른 방법은 어떤지 제안하는 방법이라는 것을 기억하자.
2. ‘틀렸다’ 라는 말은 쓰지 않는다. 틀린 코드는 없다. 상황에 더 적합한 코드만 있는 것이다 .
3. 답변은 최대한 빠르게 주는 것이 좋으나 hotfix가 아닌 경우 업무를 우선으로 한다.
4. 프로덕트에 치명적이지 않은 부분에 대해서 계속적인 pingpong은 자신과 타인의 시간을 낭비하는 것이다.
   commiter가 이유를 밝힌 시점에서 에러가 없다면 해당 pr에서는 더 이상 이야기를 하지 않는 것이 좋다.
   다만 컨벤션 전체에 영향을 준다면 이는 차후 타 미팅에서 다른 팀과 함께 이야기를 하도록 하자.(2번이 넘는 pingpong은 지양한다.)
5. 리뷰는 코드의 개선사항만 이야기하는 곳이 아니다. 잘된 곳이나 평소신경쓰지 못한 곳을 처리해 주었다면 그에 대한 칭찬이나 감사인사를 추가한다.
6. 만약 잘 만들어져서 쓸 이야기가 없다면 침묵보다는 LGTM👍 (Looks good to me)를 남기도록 한다.
7. 중요하지 않지만 거론하고 싶은 이슈라면 non-blocker(넘기고 가도 되는 리뷰)를 꼭 달아주자.
8. 이슈제기만으로는 commiter에게 도움이 되지 않는 경우가 많다.(이를테면 네이밍, 함수가 너무 길어 나눠야하는 경우 등) 적절한 예시나 대안, reference 등을 함께 적어준다.

가장 중요한 것은 코드리뷰가 프로덕트의 생산성을 방해해서는 안된다. 서로가 조금씩만 이해하고 겸손한 마음으로 임한다면 코드리뷰는 의미없이 주어지는 부담이 아닌 자신이 한 발 더 발전할 수 있는 시간으로 다가올 것이다.