코드 리뷰 in 뱅크샐러드 개발 문화

안녕하세요, 뱅크샐러드 BanksaladX iOS Engineer 정진규입니다.

코드 리뷰란 개발자가 작성한 코드를 다른 사람들이 검토하고 피드백을 전달하며, 다시 작성자가 반영하는 과정을 말합니다. 이 과정을 통해 서비스의 기술적인 구현 부분에 대해 공유·습득할 수 있으며, 일관된 아키텍처를 유지하고, 문제를 해결할 수 있는 새로운 관점을 발견할 수도 있습니다. 더 나아가, 잠재적인 장애·버그를 배포 전에 발견하여 전반적인 서비스 운영 비용을 절약할 수 있습니다.

코드 리뷰가 특별한 것은 아닙니다. ‘코드 리뷰’ 키워드로 구글에 검색하면 다양한 사례들을 찾아볼 수 있을 만큼 이미 코드 리뷰 프로세스를 도입한 IT 기업들이 많이 있습니다. 코드 리뷰를 통해 코드의 품질, 서비스의 품질을 높이려는 목적은 모든 회사가 같겠지만, 그렇다고 모든 회사의 코드 리뷰가 동일한 방식으로 진행되지는 않습니다. 코드 리뷰 프로세스는 팀의 규모, 업무 진행 방식, 회사의 특성에 따라 다양한 방식으로 진행되기 때문에, 코드 리뷰 프로세스를 보는 것은 그 회사의 개발 문화를 이해할 수 있는 힌트가 되기도 합니다.

뱅크샐러드의 코드 리뷰에도 조직이 Scale-up 되고 서비스의 복잡도가 상승하면서 겪을 수 있는 문제들을 해결해 온 노하우와 개발 문화가 담겨있습니다. 이 글을 통해 뱅크샐러드에서는 어떻게 코드 리뷰를 하는지, 뱅크샐러드의 기술 조직이 일하는 방식과 개발 문화 에 대해 이해하고 공감할 수 있는 기회가 되었으면 합니다.


GitHub과 비동기 커뮤니케이션

비동기 커뮤니케이션이란 동기 커뮤니케이션(실시간 커뮤니케이션)에 반대되는 개념으로 ‘요청에 실시간으로 응답하거나, 응답을 요구하지 않는 커뮤니케이션’ 을 의미합니다. 전화나 회의가 동기 커뮤니케이션에 속하고, Jira, Slack, 메일 등이 비동기 커뮤니케이션 방법에 속합니다.

뱅크샐러드는 비동기 커뮤니케이션을 통해 개인의 업무시간을 존중하며, 조직의 성장과 함께 커지는 커뮤니케이션 비용을 줄이기 위해 노력하고 있습니다. 업무 진행에 필요한 대부분의 커뮤니케이션은 Jira와 Slack 을 통해 비동기적으로 하고 있으며, ‘회의 없는 목요일’, ‘노이즈 캔슬링 헤드셋 착용 시 인터럽트 금지’ 와 같은 개인의 업무 시간을 존중하는 문화 덕분에 개발자들은 개인이 업무에 집중할 수 있는 시간에 방해받지 않고 업무에 몰두하고 있습니다.

GitHub Pull Request(이하 PR)를 코드 리뷰의 툴로 사용하는 이유도 비동기적으로 코드 리뷰 할 수 있는 환경을 만들 수 있기 때문입니다. 물론 비동기적이라는 장점 외에도 코드 리뷰 프로세스가 잘 구현되어 있다는 이유도 있습니다. PR을 통해 리뷰어의 Approve(승인), Comment(피드백 및 토론), Request Changes(수정 요청) 표현을 명확하게 할 수 있고, 리뷰 결과에 따라 메인 브랜치에 merge 할 수 있는 자격이 주어지거나, merge를 block 할 수 있는 기능을 제공합니다.

2020.10.23 ~ 2020.11.23 iOS 챕터의 PullRequest
Figure 1 | 2020.10.23 ~ 2020.11.23 iOS 챕터의 PullRequest

‘작은 PR’과 ‘실험 플랫폼’을 이용한 유연한 모바일 서비스 출시

작은 PR 규칙

뱅크샐러드의 코드 리뷰 문화가 성숙해지기 전에는 PR의 코드 라인 수에 대한 규칙이 없었습니다. 개발하는 기능의 복잡도에 따라 짧게는 코드는 수백 줄, 많게는 10,000줄 이상의 PR 이 만들어지기도 했습니다. 코드의 길이가 길어질수록 리뷰어의 집중도는 떨어질 수밖에 없었습니다. 코드를 이해하는 시간이 길어지고, 리뷰는 목표한 일정에 완료되지 못하고, 병목이 되기 시작했습니다. 코드 리뷰가 고객 임팩트를 내는 부분에 있어서 병목이 되지 않도록 ‘작은 PR 규칙’ (1개의 PR은 1,000  Line을 넘을 수 없다”) 을 정했습니다.

‘작은 PR 규칙’ 도입 초기에는 “복잡한 기능을 만드는데 1,000줄은 너무 적은 것 아닌가?”, “테스트 코드도 라인 수에 포함해야 하는가?” 등의 원칙에 대한 의문과, 여러 개의 PR을 만들어야 하는 부분이 오히려 생산성을 떨어트리지 않을까 하는 우려도 있었지만, 이후 몇 차례의 코드 리뷰를 진행하면서 규칙을 구체화해 나가고 노하우도 쌓이기 시작했습니다.

  • PullRequest, Commit의 단위는 최소의 기능 단위로 세분화한다
  • 테스트 코드는 Mock json 이 라인 수의 대부분을 차지하므로 제한을 두지 않는다.

코드 리뷰 문화가 성숙해가면서 우리는 리뷰 병목 해소조직의 확장성을 얻을 수 있었습니다. 모든 PR은 200 ~ 300줄 내외가 되고 1~2일 이내에 리뷰를 완료하여 리뷰의 병목을 해소할 수 있었습니다. 서비스의 성장과 함께 iOS 챕터의 구성원도 4명에서 8명으로 늘어났고, 개발되는 코드의 양(= PR의 양) 도 증가했지만 ‘작은 PR 규칙’ 아래 병목 없는 성장을 계속 해나가고 있습니다.

작은 PR 규칙이 없던 시절의 PR 목록
Figure 2 | 작은 PR 규칙 피드백

실험 플랫폼

뱅크샐러드는 데이터 기반의 의사결정을 통해 고객 임팩트를 만들어 내는 진정한 실험 조직입니다. 가설을 발굴하고 실험을 통해 검증하는 과정을 통해 새로운 기능을 출시하고 있습니다.

주차별 누적 실험 생성 추이
Figure 3 | 주차별 누적 실험 생성 추이

2020년 11월 기준 뱅크샐러드 iOS 앱 내에서는 50개의 크고 작은 실험들이 동시에 진행되고 있습니다. 모바일 개발자의 관점에서 50개의 실험이 동시에 진행되는 것은 50개의 대조군, 50개의 실험군의 코드를 동시에 운영하며 신규 실험의 추가, 기존 실험의 종료에 유연하게 대처할 수 있는 복잡도를 제어해야 하는 것을 의미합니다.

실험 플랫폼에서는 고객의 OS (Android, iOS)와 뱅크샐러드 앱 버전에 따라 실험군/대조군을 구분하고, 모바일 앱에서는 실험군/대조군에 따라 다른 UI 혹은 기능을 제공합니다.

// 실험 플랫폼에 의해 결정된 실험군/대조군에 의해 UI/UX를 결정합니다

if isControlGroup() { // 유저가 대조군에 속하면
    return controlGroupUI() // 기존 UI를 제공하고
} else {
    return treatmentGroupUI() // 실험군인 경우 새로운 UI를 제공합니다
}

‘작은 PR’과 ‘실험 플랫폼’의 조합

‘작은 PR’과 ‘실험 플랫폼’은 연관성이 없어 보이지만 실제로는 큰 시너지 효과를 내고 있습니다. ‘작은 PR’과 ‘실험 플랫폼’의 조합의 성공 사례는 2020년 6월 출시한 뱅크샐러드 2.0 이라고 부르는 UI / UX 개편 프로젝트입니다.

뱅크샐러드 2.0 UI/UX 개편
Figure 4 | 뱅크샐러드 2.0 UI/UX 개편

뱅크샐러드2.0은 많은 화면이 신규 개발되는 규모 있는 프로젝트였습니다. 매주 정기 배포를 병행하는 상황에서 메인 브랜치와 뱅크샐러드 2.0을 개발하는 브랜치를 효율적으로 관리할 수 있는 Git Branch 전략이 필요했습니다. 기능 단위의 Git Flow를 이용한다면 수개월 이후 완성된 뱅크샐러드 2.0브랜치와 메인 브랜치를 Merge 하는 경우 감당할 수 없는 양의 Conflict 가 예상됐습니다.

실험 플랫폼 이전의 Git Flow
Figure 5 | 실험 플랫폼 이전의 Git Flow

‘작은 PR’ 과 ‘실험 플랫폼’의 특징을 살려 Git branch 전략을 세우고 배포 복잡도를 최소화했습니다.

  • 작은 PR : 개발하고자 하는 기능을 최소 단위로 분리한 코드
  • 실험 플랫폼 : 고객이 사용하는 OS (iOS, Android), 앱 버전에 따라 실험군/대조군을 지정

두 가지 특성을 살리면 아래 그림과 같은 작업이 가능해집니다.

실험 플랫폼을 이용한 기능 개발 전략
Figure 6 | 실험 플랫폼을 이용한 기능 개발 전략

실험 기능의 개발, 실험의 진행은 칸막이(실험 플랫폼)가 있는 동일한 장소(메인 브랜치)에서 진행됩니다. 실험을 위한 전체 기능 개발이 완료되지 않더라도 최소 기능의 PR의 리뷰가 완료되면 주저 없이 메인 브랜치에 merge 합니다. 하나의 큰 기능을 작은 PR로 세분화하더라도 메인 브랜치에 Merge 되지 못하면 노후화가 일어나고 Conflict를 마주하게 되는 ‘작은 PR’의 한계를 ‘실험 플랫폼’으로 극복했습니다.

실험 플랫폼 이후의 Git Flow
Figure 7 | 실험 플랫폼 이후의 Git Flow

뱅크샐러드2.0 프로젝트는 2020.04.01 ~ 2020.06.01 기간 동안 , + 6만 -3만 라인의 변경점의 규모 있는 코드를 약 800 Commit으로 세분화/개발하여 Conflict 와 리뷰로 인한 병목 없이 고객에게 성공적으로 오픈할 수 있었습니다.


저 문맥(Low Context) 커뮤니케이션을 지향하는 문화

뱅크샐러드는 저 문맥(Low Context) 커뮤니케이션을 지향합니다. 질문을 하거나 대답을 할 때, 내가 알고 있는 것을 상대방도 알고 있을 것이라는 가정을 하지 않고 충분한 문맥을 전달해야 한다는 의무를 가집니다. 상대방의 의도를 파악하기 위한 추가적인 커뮤니케이션 또는 미스 커뮤니케이션으로 발생하는 비용을 매우 비싼 비용이라는 문제의식을 갖고 있습니다.

코드 리뷰 과정에서 리뷰의 요청과 피드백도 충분한 문맥을 제공해야 한다는 원칙을 가져가고 있습니다. 리뷰 요청자는 리뷰어가 사전 지식이 없는 상태에서 리뷰에 참여한다는 가정으로 모든 문맥을 제공합니다. 뱅크샐러드 iOS 챕터에서는 리뷰어가 PR에 대한 충분한 문맥을 이해할 수 있는 항목들을 정의했고, GitHub Template을 활용하여 코드 리뷰 프로세스의 일부로 자리 잡도록 했습니다.

리뷰 요청자의 준비물

  • 제품 기능 기획
  • 기술적인 구현 계획
  • 현재 PR에서의 변경점
  • 예상 / 구현된 디자인
  • 어떤 내용들을 테스트했는지 / 해야 하는지 목록
GitHub PR의 예시
Figure 8 | GitHub PR의 예시

PR 이 Open 되기까지 참고했던 프로덕트 스펙(기획서), 테크 스펙, Figma(디자인 협업 도구) 의 링크를 첨부하면서 간단하지만 리뷰어에게 PR에 대한 모든 Context를 공유하고 있습니다. 복잡한 유저 액션이 포함된 PR의 경우 GIF 를 적극 활용하여 리뷰어의 이해를 돕고 있습니다.


코드 리뷰에 임하는 자세

코드 리뷰 과정에서 이루어지는 것

  • 일관된 아키텍처를 유지하고 있는지
  • 다른 해결 방법에 대한 의견 제시
  • 버그가 발생할 수 있는 가능성 제시
  • 기술적인 지식, 노하우 공유
  • 히스토리 전달
가독성 리뷰
Figure 9 | 가독성 리뷰
기술부채를 줄이기 위한 리뷰
Figure 10 | 기술부채를 줄이기 위한 리뷰

순리

빠르게 기능을 런칭하고 매출을 올리는 것 vs 개발 기간이 조금 걸리더라도 이상적인 설계대로 개발하는 것

“if 문 하나만 넣으면 빠르게 해결할 수 있는데, 구조변경 or 리팩토링까지 해야 하나? 그냥 하면 안 되나?”

개발자들은 한 번쯤 해봤을 법한 고민일 겁니다. 현실에 타협하거나 하지 않거나. 뱅크샐러드 기술 조직은 타협하지 않습니다. 우리는 어떤 것이 정답일지 고민하는 과정에서 항상 순리 를 외칩니다. ‘순리’란 이치나 도리를 따르는 것을 말합니다. 개발 관점에서의 순리는 문제를 해결하는 방법 중 Best Practice 를 찾아나가는 과정이라고 생각합니다. 타협하지 않고 순리대로 가는 것이 결국 가장 빠른길임을 모두가 공감하고 지키려고 노력하고있습니다.

순리
Figure 11 | 순리

커뮤니케이션 비용을 줄이기 위한 Pn 룰

우리는 상대방과 대화를 할 때, ‘말’이라는 언어적 표현 외에도 ‘표정’, ‘손짓’ 등의 비언어적 표현도 적극적으로 활용합니다. 온라인으로 코드 리뷰를 진행하는 경우 ‘글’ 로만 생각, 감정을 포함한 의견을 전달하기 때문에 전달력이 상대적으로 부족하고, 이로 인해 가벼운 농담이 상대방에게 진지한 이야기로 전달되는 상황이 연출되어 Blocker 가 될 수도 있습니다.

뱅크샐러드 기술 조직은 코드 리뷰의 코멘트에 Pn 룰을 사용하여 리뷰어가 코멘트를 강조하고 싶은 정도를 표현합니다.

  • P1: 꼭 반영해주세요 (Request changes)

리뷰어는 PR의 내용이 서비스에 중대한 오류를 발생할 수 있는 가능성을 잠재하고 있는 등 중대한 코드 수정이 반드시 필요하다고 판단되는 경우, P1 태그를 통해 리뷰 요청자에게 수정을 요청합니다. 리뷰 요청자는 p1 태그에 대해 리뷰어의 요청을 반영하거나, 반영할 수 없는 합리적인 의견을 통해 리뷰어를 설득할 수 있어야 합니다.

  • P2: 적극적으로 고려해주세요 (Request changes)

작성자는 P2에 대해 수용하거나 만약 수용할 수 없는 상황이라면 적합한 의견을 들어 토론할 것을 권장합니다.

  • P3: 웬만하면 반영해 주세요 (Comment)

작성자는 P3에 대해 수용하거나 만약 수용할 수 없는 상황이라면 반영할 수 없는 이유를 들어 설명하거나 다음에 반영할 계획을 명시적으로(JIRA 티켓 등으로) 표현할 것을 권장합니다. Request changes 가 아닌 Comment 와 함께 사용됩니다.

  • P4: 반영해도 좋고 넘어가도 좋습니다 (Approve)

작성자는 P4에 대해서는 아무런 의견을 달지 않고 무시해도 괜찮습니다. 해당 의견을 반영하는 게 좋을지 고민해 보는 정도면 충분합니다.

  • P5: 그냥 사소한 의견입니다 (Approve)

작성자는 P5에 대해 아무런 의견을 달지 않고 무시해도 괜찮습니다.

FYI) Pn 룰은 뱅크샐러드에서 성장하는 회사에서 커뮤니케이션은 가장 큰 비용이라는 문제의식에서 이를 해결하기 위한 문화를 배경으로 탄생했습니다. Pn 룰은 코드 리뷰 외에도 슬랙 등 텍스트로 커뮤니케이션을 하는 곳에서도 활용하고 있습니다

Slack에서의 Pn룰
Figure 12 | Slack에서의 Pn룰

리뷰 우선순위 판단을 돕는 D-n 룰

코드 생산성이 폭발하는 날, 하루에 여러 개의 PR 리뷰 요청이 오기도 합니다. 쌓여있는 코드 리뷰 요청 목록은 고객 임팩트만 보고 달려가는 개발자들의 마음을 불편하게 합니다.

리뷰어는 리뷰를 요청하는 시점에 PR 이 Merge 되어야 하는 일정을 공유하여 리뷰어가 Working day 안에서 스스로 우선순위를 결정하고 개발에 집중할 수 있는 시간을 환경을 만들어 나가고 있습니다.

D-0 사례
Figure 13 | D-0 사례
  • D-0 (ASAP)

긴급한 수정사항으로 바로 리뷰해 주세요. 앱의 오류로 인해 장애가 발생하거나, 빌드가 되지 않는 등 긴급 이슈가 발생할 때 사용합니다.

  • D-N (Within N days)

“Working Day 기준으로 N일 이내에 리뷰해 주세요”

일반적으로 D-2 태그를 많이 사용하며, 중요도가 낮거나 일정이 여유 있는 경우 D-3 ~ D-5 를 사용하기도 합니다.


인건비가 가장 비싼것

코딩 스타일 ( Indent, Convention )

코딩 스타일은 가독성 관점에서 포기할 수 없는 부분입니다. 하지만 코드 리뷰 단계에서 Blocker 가 될 만큼 중요하지는 않습니다. 신규 입사자의 온보딩 과정에서
코딩 스타일을 유지하면서도 우리의 리소스를 지키자라는 고민의 결과로 “자동화 할 수 있는 부분은 자동화하여 사람의 리소스를 아끼자” 라는 답을 얻었습니다.

코딩 스타일의 자동화 관련하여 리서치 결과 SwiftFormat을 도입했고, 스타일 규칙을 팀 내의 합의된 코딩 스타일을 룰로 정했습니다. 빌드 Phase에 SwiftFormat을 실행시켜 자동으로 코딩 스타일을 맞춰지는 프로세스를 만들었습니다. 개발자는 코딩 스타일에 대한 부담감을 줄이고 리뷰어는 리뷰 단계에서 코딩 스타일이라는 관점을 배제하면서 더 효율적인 리뷰가 될 수 있도록 하고 있습니다.

유닛 테스트

안정적인 서비스 운영을 위해 테스트 코드 작성을 높은 수준으로 권장하고 있으며, 테스트 케이스를 통해 코드의 변경점으로 인해 발생할 수 있는 사이드 이펙트를 제거하고 있습니다. iOS의 유닛 테스트를 실행하기 위해서는 의존성 설치, 빌드, 테스트의 과정이 필요하고 뱅크샐러드 iOS 앱은 약 10~15분 정도의 짧지 않은 시간이 소요됩니다. 그 시간 동안 다른 기능 개발을 할 수 없으므로(= 다른 Repository에서 작업은 가능하나 효율적이지 않으므로) 하루에 4번만 테스트 코드를 실행해도 1시간의 개발 시간이 줄어들게 됩니다.

개발자의 생산성 향상을 근거로 유닛 테스트만 담당하는 고 사양의 맥프로를 구매하여 Jenkins 를 이용해 CI를 구축했습니다. GitHub의 Pull Request Opened, Commit Changed를 Trigger로 테스트 코드를 실행하며, 그 결과를 PR 의 status 에 반영하는 과정을 자동화 했습니다. 유닛 테스트 결과가 Failure 인 경우 Merge 가 Block 되어 유닛 테스트가 통과하지 못하는 PR 은 반드시 테스트 코드가 실패한 원인을 파악하고 수정해야만 합니다.

유닛 테스트 실패 Case
Figure 14 | 유닛 테스트 실패 Case
뱅크샐러드 iOS의 유닛 테스트 현황
Figure 15 | 뱅크샐러드 iOS의 유닛 테스트 현황

유닛 테스트 자동화를 통해 iOS 챕터 구성원은 테스트 수행에 부담을 느끼지 않습니다. 개발과 테스트 코드 작성에만 전념하고 PR을 만들면, PR에 대한 테스트 결과를 자동으로 받아볼 수 있기 때문입니다. 뱅크샐러드 iOS앱은 2020년 9월 기준 약 800회의 유닛 테스트가 실행되었습니다. 시간으로 환산하면 약 8,000분(약 =133시간)에 해당하며 iOS 개발자 한 명의 한 달 업무시간(= 약 160시간)과 비슷합니다. 유닛 테스트 자동화로 1명의 iOS 개발자를 채용한 효과를 얻고 있습니다.



마치며

뱅크샐러드의 코드 리뷰에는 기술 조직이 일하는 방식, 문화가 녹아 있습니다.

이 글을 통해 뱅크샐러드에서 왜 코드 리뷰를 하는지, 뱅크샐러드의 기술 조직이 일하는 방식이 무엇인지 이해하고 공감할 수 있는 기회가 되었으면 합니다.

보다 빠르게 뱅크샐러드에 도달하는 방법 🚀

지원하기