InfoGrab DocsInfoGrab Docs

코드 리뷰 가이드라인

요약

GitLab CE와 EE의 모든 머지 리퀘스트는 코드가 효과적이고 이해하기 쉬우며 유지 보수가 가능하고 안전한지 확인하기 위해 코드 리뷰를 거쳐야 합니다. 시작하기 전에 기여 승인 기준을 숙지합니다. 코드는 소속 그룹의 리뷰어 또는 도메인 전문가에게 리뷰를 받습니다.

GitLab CE와 EE의 모든 머지 리퀘스트는 코드가 효과적이고 이해하기 쉬우며 유지 보수가 가능하고 안전한지 확인하기 위해 코드 리뷰를 거쳐야 합니다.

머지 리퀘스트 리뷰, 승인, 병합 받기#

시작하기 전에 기여 승인 기준을 숙지합니다.

코드는 소속 그룹의 리뷰어 또는 도메인 전문가에게 리뷰를 받습니다.

작고 단순한 변경 사항이라면 리뷰어 단계를 건너뛰고 곧바로 메인테이너에게 전달할 수 있습니다. 작고 단순한 변경 사항의 예는 다음과 같습니다.

  • 오타 수정이나 간단한 문구 변경.
  • 동작을 바꾸지 않는 소규모 리팩토링.
  • 한 달 이상 기본값으로 활성화되어 있던 기능 플래그 제거.
  • 사용하지 않는 메서드나 클래스 제거.
  • 다섯 줄 미만의 코드 변경으로 끝나는, 충분히 이해된 로직 변경.

그 밖의 경우에는 MR 이 건드리는 카테고리별로 리뷰어를 지정한 뒤 메인테이너에게 전달합니다. 보안 관련 지원이 필요하면 @gitlab-com/gl-security/appsec을 포함합니다.

리뷰어가 승인하면 메인테이너가 리뷰한 뒤 병합합니다. 마지막 필수 승인자가 병합합니다.

CODEOWNERS가 요구하는 승인은 일반 승인보다 도메인별 승인을 먼저 받습니다. 도메인별 승인자가 메인테이너를 겸하는 경우 두 관점을 함께 리뷰하고 한 번만 승인합니다.

승인 가이드라인#

아래 메인테이너의 책임 절에서 설명하듯이 머지 리퀘스트는 도메인 전문성을 갖춘 메인테이너가 승인하고 병합하도록 하는 것을 권장합니다. 첫 리뷰어의 선택적 승인은 여기서 다루지 않습니다. 다만 머지 리퀘스트는 메인테이너에게 전달하기 전에 개요 절에서 설명한 대로 리뷰어의 리뷰를 받아야 합니다.

머지 리퀘스트에 포함된 내용 필요한 승인자
~backend 변경 사항 1 백엔드 메인테이너.
~database 마이그레이션 또는 비용이 큰 쿼리 변경 사항 2 데이터베이스 메인테이너. 자세한 내용은 데이터베이스 리뷰 가이드라인을 참고합니다.
~workhorse 변경 사항 Workhorse 메인테이너.
~frontend 변경 사항 1 프론트엔드 메인테이너.
~UX 사용자에게 보이는 변경 사항 3 Product Designer. 자세한 내용은 디자인 및 사용자 인터페이스 가이드라인을 참고합니다.
새 JavaScript 라이브러리 추가 1 - 라이브러리가 번들 크기를 크게 늘리는 경우 Frontend Design System 구성원.
- 새 라이브러리가 사용하는 라이선스가 GitLab에서 사용 승인을 받지 않은 경우 법무 부서 구성원.

라이선스 호환성에 대한 자세한 내용은 GitLab 라이선스 및 호환성 문서에서 확인할 수 있습니다.
새 의존성 또는 파일 시스템 변경 - Distribution 팀 구성원. 자세한 내용은 Distribution 팀과 협업하는 방법을 참고합니다.
- RubyGems의 경우 AppSec 리뷰를 요청합니다.
~documentation 또는 ~UI text 변경 사항 해당 DevOps Stage 그룹의 배정에 따른 테크니컬 라이터.
개발 가이드라인 변경 리뷰 프로세스를 따라 그에 맞는 승인을 받습니다.
.ai/ 아래 AI 지시 파일 변경 AI 하네스 DRI. 자세한 내용은 AI 지시 파일 리뷰 가이드라인을 참고합니다.
엔드투엔드 변경과 엔드투엔드 이외 변경 4 Software Engineer in Test.
엔드투엔드 변경만 있는 경우 4 또는 MR 작성자가 Software Engineer in Test 인 경우 Quality 메인테이너.
새로 추가하거나 업데이트한 애플리케이션 제한 프로덕트 매니저.
Analytics Instrumentation(텔레메트리 또는 분석) 변경 사항 Analytics Instrumentation 엔지니어.
GitLab에 추가되는 새 서비스(Puma, Sidekiq, Gitaly 등) 프로덕트 매니저. 자세한 내용은 GitLab에 서비스 컴포넌트를 추가하는 절차를 참고합니다.
인증과 관련된 변경 사항 Manage:Authentication. 자세한 내용은 그룹 페이지의 코드 리뷰 절을 확인합니다. 이 팀의 리뷰가 필요한 것으로 알려진 파일 패턴은 CODEOWNERS 파일의 Authentication 절에 나열되어 있으며, 이 파일을 수정하는 모든 머지 리퀘스트의 승인자 목록에 해당 팀이 표시됩니다.
커스텀 역할 또는 정책과 관련된 변경 사항 Manage:Authorization Engineer.
  1. JavaScript 스펙을 제외한 스펙은 ~backend 코드로 봅니다. Haml 마크업은 ~frontend 코드로 봅니다. 다만 Haml 템플릿 안의 Ruby 코드는 ~backend 코드로 봅니다. 판단이 어려우면 프론트엔드와 백엔드 리뷰를 모두 요청합니다.

    Haml 템플릿 변경의 경우에는 다음과 같이 요청합니다.

    • 템플릿에 Ruby 로직, 메서드 호출, 변수 할당, 조건문, 루프, 데이터 준비, 보안 검사 등 서버 측 처리가 포함된 변경이라면 백엔드 리뷰를 요청합니다.
    • DOM 구조, CSS 클래스, HTML 속성, 접근성 기능, 사용자 상호작용, 반응형 디자인, 시각적 표현에 영향을 주는 변경이라면 프론트엔드 리뷰를 요청합니다.
    • Ruby 로직과 상당한 UI 수정이 함께 들어간 복잡한 변경이거나, 백엔드와 프론트엔드가 서로 얽혀 있는 경우(예: 백엔드가 Vue 나 JavaScript에서 사용하는 데이터를 제공하는 경우)에는 백엔드 기능과 프론트엔드 사용자 경험이 모두 제대로 평가되도록 두 리뷰를 모두 요청합니다.
      • 예: Vue.js 컴포넌트에 전달할 데이터 속성을 준비하기 위해 Ruby 메서드를 호출하는 Haml 템플릿(예: project_id: @project&.to_global_id)이라면 Ruby 로직의 정확성은 백엔드 리뷰가, 컴포넌트 통합은 프론트엔드 리뷰가 도움이 됩니다.
  2. 머지 리퀘스트가 비용이 큰 쿼리를 도입할 가능성이 있으면 데이터베이스 메인테이너의 안내를 받는 것이 좋습니다. 해당 코드 줄에 SQL 쿼리와 함께 댓글을 남기면 조언을 받기에 가장 효율적입니다.

  3. 사용자에게 보이는 변경에는 정도와 무관하게 모든 시각적 변경과, 스크린 리더가 콘텐츠를 읽는 방식에 영향을 주는 렌더링된 DOM 변경이 함께 포함됩니다. 전담 Product Designer가 없는 그룹은 커뮤니티 기여가 아닌 한 기능 변경에 Product Designer의 승인을 받지 않아도 됩니다.

  4. 엔드투엔드 변경에는 qa 디렉터리의 모든 파일이 포함됩니다.

머지 리퀘스트 리뷰#

변경이 필요한 이유(버그 수정, 사용자 경험 개선, 기존 코드 리팩토링)를 먼저 파악합니다. 그다음에는 아래 사항을 따릅니다.

  • 반복 횟수를 줄이기 위해 꼼꼼하게 확인합니다.
  • 강하게 주장하는 의견과 그렇지 않은 의견을 구분해 전달합니다.
  • 문제를 해결하면서도 코드를 단순화할 방법을 찾습니다.
  • 대안 구현을 제시하되 작성자가 이미 그 방법을 검토했다고 가정합니다. ("여기에 커스텀 validator를 쓰는 방안은 어떻습니까?")
  • 작성자의 관점을 이해하려고 노력합니다.
  • 브랜치를 체크아웃해 변경 사항을 로컬에서 테스트합니다. GDK를 크게 수정해야 하는 MR 이라면 대신 스크린샷, 동영상, 도메인 전문가의 검증을 요청하는 방안을 고려합니다. 테스트 과정에서 자동화된 테스트를 추가할 기회를 발견할 수 있습니다.
  • 이해하지 못한 코드가 있으면 그렇다고 말합니다.
  • 의도를 전달할 때는 Conventional Comment 형식을 사용합니다. 필수가 아닌 제안은 (**non-blocking:**)으로 표시합니다. non-blocking 제안만 남았다면 기다리지 않고 MR을 다음 단계로 넘깁니다.
  • 열려 있는 의존성이 없는지 확인합니다. 차단 요소가 있는지 연결된 이슈를 확인합니다. 열려 있는 MR 때문에 막혀 있다면 MR 의존성을 설정합니다.
  • 줄 단위 의견을 남긴 뒤에는 "제가 보기에는 괜찮습니다" 나 "몇 가지만 반영하면 됩니다" 같은 요약 의견을 남깁니다.
  • 리뷰 결과 변경이 필요하면 작성자에게 알립니다.
Warning

머지 리퀘스트가 포크에서 왔다면 커뮤니티 기여 추가 가이드라인도 확인합니다.

GitLab 특정 고려 사항#

GitLab은 Omnibus 패키지부터 소스 설치까지 다양한 환경에서 사용됩니다. GitLab.com 자체도 대규모 Enterprise Edition 인스턴스입니다. 여기에서 다음과 같은 사항이 따라옵니다.

  1. 쿼리 변경은 GitLab.com 규모에서 성능이 나빠지지 않는지 테스트해야 합니다. 데이터베이스 리뷰 가이드라인을 참고합니다.
  2. 데이터베이스 마이그레이션은 다음 조건을 충족해야 합니다.
    1. 되돌릴 수 있어야 합니다.
    2. GitLab.com 규모에서 성능이 충분해야 합니다. 확실하지 않으면 메인테이너에게 스테이징 환경에서 마이그레이션을 테스트해 달라고 요청합니다.
    3. 올바른 마이그레이션 유형이어야 합니다. 어떤 마이그레이션 유형을 고를지는 안내를 참고합니다.
  3. Sidekiq 워커는 하위 호환되지 않는 방식으로 변경할 수 없습니다.
  4. 캐시된 값은 릴리스가 바뀌어도 남아 있을 수 있습니다. 캐시된 값이 반환하는 유형을 바꾸는 경우(예: 문자열이나 nil에서 배열로) 캐시 키도 함께 변경합니다.
  5. 설정은 최후의 수단으로만 추가합니다. GitLab Rails에 새 설정 추가를 참고합니다.
  6. 파일 시스템 액세스는 클라우드 네이티브 아키텍처에서 사용할 수 없습니다. 파일을 저장해야 한다면 오브젝트 스토리지를 지원하는지 확인합니다. 자세한 내용은 업로드 문서를 참고합니다.

머지 리퀘스트 작성자의 책임#

작성자는 최선의 해법을 찾는 직접 책임자 (DRI)입니다. 리뷰 수명 주기 동안 담당자로 남아 있습니다. 스스로를 담당자로 지정할 수 없으면 리뷰어에게 지정을 요청합니다.

너무 크거나 여러 이슈를 함께 수정하거나 복잡도가 높거나 두 개 이상의 기능을 구현하는 머지 리퀘스트는 제출하지 않습니다.

MR 이 여러 CODEOWNERS 절을 건드린다면 필요한 승인을 줄이기 위해 관심사별로 MR을 나눠 리뷰를 병렬로 진행하는 방안을 고려합니다. 메인테이너 리뷰를 요청하기 전에 다음을 확인합니다.

  • MR 이 의도한 문제를 가장 적절한 방식으로 해결하는지.
  • 모든 요구 사항을 충족하는지.
  • 남아 있는 버그, 로직 문제, 다루지 않은 예외 사례, 알려진 취약점이 없는지.

코드 리뷰 가이드라인에 따라 MR을 스스로 리뷰합니다. 결정이나 절충이 있었던 줄, 또는 리뷰어가 코드를 이해하는 데 컨텍스트가 필요한 줄에는 인라인 댓글을 추가합니다.

필요에 따라 도메인 전문가, 프로덕트 매니저, UX 디자이너, 데이터베이스 전문가를 참여시킵니다. MR에 도메인 전문가 리뷰가 필요한지 확실하지 않다면 필요한 것입니다.

MR 이 10개 이상인 기능이라면 EM 이나 Staff Engineer와 함께 컨텍스트를 공유하는 고정 메인테이너를 정합니다.

MR 이 여러 도메인을 건드린다면 도메인마다 전문가의 리뷰를 요청합니다.

리뷰를 요청하기 전에 다음 항목에 대해 MR diff 댓글을 추가합니다.

  • 추가한 린팅 규칙(RuboCop, JS 등).
  • 추가한 라이브러리(Ruby gem, JS 라이브러리 등).
  • 분명하지 않은 상위 클래스나 메서드로 이어지는 링크.
  • 벤치마크 결과.
  • 보안상 문제가 될 수 있는 코드.

리뷰어가 해법을 검증하는 데 필요한 프로젝트, 스니펫, 자산에 접근할 수 있는지 확인합니다.

리뷰어를 지정할 때는 각 리뷰어가 어느 도메인에 집중해야 하는지 댓글로 알립니다. 팀원이 여러 영역에 전문성을 갖춘 경우의 모호함을 없앨 수 있습니다. 예시는 MR 75921 과 MR 109500을 참고합니다.

소스 코드의 TODO 주석은 리뷰어가 요구할 때만 추가합니다. TODO를 추가한다면 관련 이슈 링크를 포함합니다.

주석은 코드가 무엇을 하는지만이 아니라 왜 그런지 설명하도록 작성합니다.

메인테이너 리뷰는 테스트가 통과한 뒤에만 요청합니다. 테스트가 실패했다면 그 이유를 댓글로 설명합니다. 메인테이너에게 이메일이나 Slack으로 연락하는 것은 즉시 처리해야 하는 요청에만 한정합니다. 그 밖의 경우에는 리뷰어로 추가하는 것으로 충분합니다.

리뷰어의 책임#

리뷰어는 선택된 해법의 세부 내용을 리뷰할 책임이 있습니다.

리뷰 응답 SLO 안에 리뷰할 수 없다면 작성자에게 알리고, 리뷰 업무량 대시보드에서 대체 리뷰어를 찾아 지정합니다.

MR 이 모든 기여 승인 기준을 충족한다고 확신하면 다음을 진행합니다.

  1. Approve를 선택합니다.
  2. 작성자에게 알리기 위해 @로 멘션합니다.
  3. 도메인 전문성을 갖춘 메인테이너에게 리뷰를 요청하거나 리뷰어 룰렛의 추천을 따릅니다.

메인테이너의 책임#

메인테이너는 GitLab 코드베이스 전반의 건전성, 품질, 일관성에 대한 책임을 집니다. 리뷰에서는 아키텍처, 코드 구성, 관심사 분리, 테스트, DRY 여부, 일관성, 가독성에 초점을 맞춥니다.

메인테이너는 MR 이 승인 기준을 합리적으로 충족하는지 확인하는 DRI 입니다.

메인테이너는 MR의 영향을 평가할 때 건전한 판단을 내립니다. MR을 병합할 수 없다고 판단하면 그 사실을 말하는 것도 메인테이너의 책임입니다. 또한 메인테이너는 다른 사람의 의견을 들어야 할 시점을 아는 전문 조언자 역할도 합니다.

메인테이너가 MR을 승인하면 작성자와 함께 책임을 지게 됩니다. 따라서 프로덕션 사고가 발생하면 문제 해결을 돕기 위해 메인테이너가 호출될 수 있습니다.

일부 머지 리퀘스트는 stable 브랜치를 대상으로 합니다. 이런 요청을 처리하는 방법은 패치 릴리스 런북에서 확인합니다.

모범 사례#

도메인 전문가#

도메인 전문가는 특정 기술, 제품 기능, 코드베이스 영역에 상당한 경험을 갖춘 팀원입니다. 팀원은 스스로 도메인 전문가임을 밝히고 이를 팀 프로필에 추가하는 것이 좋습니다.

스스로 도메인 전문가임을 밝힐 때는 .yml 파일을 변경하는 MR을 이미 인정받은 도메인 전문가나 담당 Engineering Manager가 병합하도록 지정하는 것이 좋습니다.

자동으로 도메인 전문가로 간주되는 경우에 대해서는 다음과 같이 가정합니다.

  • 특정 스테이지/그룹(예: create: source code)에서 일하는 팀원은 자신이 담당하는 애플리케이션 영역의 도메인 전문가로 간주합니다.
  • 특정 기능(예: search)을 담당하는 팀원은 그 기능의 도메인 전문가로 간주합니다.

코드 리뷰는 기본적으로 도메인 전문성을 갖춘 팀원에게 배정합니다. UX 리뷰는 기본적으로 Review Roulette가 추천한 리뷰어에게 배정합니다. 디자이너 가용 인력 제한으로 Product Designer가 지원하지 않는 영역은 커뮤니티 기여가 아닌 한 UX 리뷰를 요구하지 않습니다. 적합한 도메인 전문가가 없으면 다른 팀원에게 MR 리뷰를 요청하거나 리뷰어 룰렛의 추천을 따를 수 있습니다(UX 리뷰는 위 내용을 참고합니다). 지정하기 전에 해당 팀원이 부재 중인지 다시 확인합니다.

도메인 전문가를 찾는 방법은 다음과 같습니다.

  • Merge Request 승인 위젯에서 View eligible approvers를 선택합니다. 이 위젯은 코드베이스 영역별로 권장 승인과 필수 승인을 보여 줍니다. 이 규칙은 Code Owners에 정의되어 있습니다.
  • 머지 리퀘스트와 관련된 스테이지 또는 그룹에서 일하는 팀원 목록을 확인합니다.
  • engineering projects 페이지나 GitLab 팀 페이지에서 팀원의 도메인 전문성을 확인합니다. 도메인은 본인이 밝힌 것이므로, 머지 리퀘스트의 변경 사항을 도메인에 연결할 때는 스스로 판단합니다.
  • 머지 리퀘스트의 파일에 기여한 팀원을 찾습니다. git log <file>을 실행해 로그를 확인합니다.
  • 해당 파일을 리뷰한 팀원을 찾습니다. 관련 머지 리퀘스트는 다음 방법으로 찾습니다.
    1. git log <file>로 커밋 SHA를 확인합니다.
    2. https://gitlab.com/gitlab-org/gitlab/-/commit/로 이동합니다.
    3. 커밋에 표시된 관련 머지 리퀘스트를 선택합니다.

리뷰어 룰렛#

Note

리뷰어 룰렛은 GitLab.com을 위한 내부 도구이며 고객 설치 환경에서는 사용할 수 없습니다.

Danger bot은 MR 이 건드리는 코드베이스 영역마다 리뷰어와 메인테이너를 고릅니다. 더 적합한 사람을 알고 있다면 추천을 대신해 직접 지정합니다.

GitLab은 룰렛 추천 표의 Reviewer 칼럼이 여전히 유용한지 평가하는 실험을 진행하고 있습니다. CI/CD 변수로 제어되는 일부 머지 리퀘스트에서는 Danger가 Reviewer 칼럼을 숨기고 Category와 Maintainer 칼럼만 표시합니다. 해당되는 MR 이라도 위에 링크된 리뷰 업무량 대시보드로 리뷰어를 찾을 수 있습니다. 실험에 포함된 MR에는 결과를 측정할 수 있도록 roulette-experiment::reviewer-column-hidden 또는 roulette-experiment::reviewer-column-shown 레이블이 지정됩니다.

룰렛은 상태에 OOO, PTO, Parental Leave, Friends and Family, Conference가 포함된 사람과 리뷰 가능 한도에 도달한 사람(숫자 상태 이모지 2️⃣-5️⃣ 로 설정)을 건너뜁니다.

승인 체크리스트#

이 체크리스트는 머지 리퀘스트(MR)의 작성자, 리뷰어, 메인테이너가 품질, 성능, 신뢰성, 보안, 가관측성, 유지 보수성에 큰 영향을 주는 위험 요소를 분석했는지 확인하도록 돕습니다.

체크리스트를 사용하면 소프트웨어 엔지니어링의 품질이 높아집니다. 이 체크리스트는 GitLab 코드베이스 기여자의 역량을 뒷받침하는 간단한 도구입니다.

품질#

품질 가이드라인에 대한 자세한 내용은 테스팅을 참고합니다.

  1. 코드 리뷰 가이드라인에 따라 이 MR을 스스로 리뷰했습니다.
  2. 코드가 소프트웨어 설계 가이드라인을 따릅니다.
  3. 테스트 피라미드에 따라 자동화된 테스트가 있는지 확인합니다. 빠진 테스트를 추가하거나 테스트 공백을 기록한 이슈를 만듭니다.
  4. GitLab.com, Dedicated, self-managed에 미치는 기술적 영향을 검토했습니다.
  5. 이 변경이 시스템의 프론트엔드, 백엔드, 데이터베이스 부분에 미치는 영향을 적절히 검토하고 ~ux, ~frontend, ~backend, ~database 레이블을 그에 맞게 적용했습니다.
  6. 지원되는 모든 브라우저에서 이 MR을 테스트했거나, 이 테스트가 필요하지 않다고 판단했습니다.
  7. 이 변경이 업데이트 간 하위 호환성을 유지하는지 확인했거나, 해당하지 않는다고 판단했습니다.
  8. EE 콘텐츠가 있다면 FOSS와 올바르게 분리했습니다. FOSS 컨텍스트에서 CI 파이프라인 실행을 고려합니다.
  9. 기존 데이터가 예상보다 다양할 수 있다는 점을 검토했습니다. 예를 들어 새 모델 유효성 검사를 추가한다면 기존 데이터에는 선택 적용하는 방안을 고려합니다.
  10. 이 MR과 관련된 불안정한 테스트를 수정했거나, 무시해도 되는 이유를 설명했습니다. 불안정한 테스트에는 Flaky test '<path/to/test>' was found in the list of files changed by this MR. 오류가 있지만, 경고와 함께 통과하는 job에 있을 수도 있습니다.

성능, 신뢰성 및 가용성#

  1. 이 MR 이 성능을 해치지 않는다고 확신하거나, 성능 영향 평가를 리뷰어에게 요청했습니다. (머지 리퀘스트 성능 가이드라인)
  2. MR 설명에 데이터베이스 리뷰어를 위한 정보를 추가했거나, 필요하지 않다고 판단했습니다.
  3. 이 변경의 가용성 및 신뢰성 위험을 검토했습니다.
  4. 향후 예상 성장에 따른 확장성 위험을 검토했습니다.
  5. 평균적인 고객보다 데이터가 훨씬 많을 수 있는 대규모 고객에 이 변경이 미치는 성능, 신뢰성, 가용성 영향을 검토했습니다.
  6. 최소 시스템 요건에서 GitLab을 운영하는 고객에 이 변경이 미치는 성능, 신뢰성, 가용성 영향을 검토했습니다.
  7. 이 변경이 Cells 아키텍처와 호환된다고 확신합니다. 자세한 내용은 Cells 개발 원칙을 참고합니다.

가관측성 계측#

  1. 가관측성을 통해 디버깅과 선제적 성능 개선이 가능하도록 충분한 계측을 포함했습니다. 기능 플래그, 로깅, 계측을 추가한 예시를 참고합니다.

문서#

  1. changelog 트레일러를 포함했거나, 필요하지 않다고 판단했습니다.
  2. 문서를 추가하거나 업데이트했거나, 이 MR에는 문서 변경이 필요하지 않다고 판단했습니다.

보안#

  1. 이 MR에 자격 증명이나 토큰의 처리·저장, 인가 및 인증 메서드, 그 밖에 보안 리뷰 가이드라인에 설명된 항목의 변경이 포함된다면 ~security 레이블을 추가하고 @gitlab-com/gl-security/appsec을 @로 멘션했습니다.
  2. 보안 리뷰를 언제 어떻게 요청하는지에 대해 내부 애플리케이션 보안 리뷰 문서를 확인했고, 이 변경에 보안 리뷰가 필요하다고 판단해 리뷰를 요청했습니다.
  3. 머지 리퀘스트 승인 정책으로 인해 MR을 막고 있는 보안 스캔 결과가 있는 경우:
    • 참 양성 결과는 머지 리퀘스트를 병합하기 전에 수정합니다. 수정하면 머지 리퀘스트 승인 정책이 요구하는 AppSec 승인이 해제됩니다.
    • 거짓 양성 결과, 위험 수용을 논의해야 하는 사항, 판단이 애매한 사항은 @gitlab-com/gl-security/appsec에 문의합니다.

배포#

  1. 이 변경이 위험도가 높을 수 있으므로 기능 플래그 사용을 검토했습니다.
  2. 기능 플래그를 사용한다면 프로덕션에서 테스트하기 전에 스테이징에서 변경을 테스트할 계획이며, 모든 고객에게 배포하기 전에 일부 프로덕션 고객에게 먼저 배포하는 방안을 검토했습니다.
  3. 완료의 정의에 따라 기본 설정 또는 새 설정 변경을 Infrastructure 부서에 알렸거나, 필요하지 않다고 판단했습니다.

규정 준수#

  1. 올바른 MR 유형 레이블이 적용되었는지 확인했습니다.

코드 리뷰 참여#

  • 친절하게 대합니다.
  • 프로그래밍 결정의 상당수가 의견임을 받아들입니다. 절충점을 논의하고 빠르게 정리합니다.
  • 질문을 합니다. 요구가 아니라 제안을 합니다.
  • 명확하게 표현합니다. 온라인에서는 의도가 늘 전달되지는 않습니다.
  • 겸허한 태도를 유지합니다. 오해가 길어지면 일대일 통화를 고려하고 이후 요약을 남깁니다.
  • 특정 사람에게 하는 댓글이라면 그 사람을 직접 멘션합니다.
  • 처음 푸시하기 전에 전체 diff를 읽습니다. 관련 없는 변경과 디버그 코드가 있는지 확인합니다.
  • 머지 리퀘스트 가이드라인에 따라 상세한 설명을 작성합니다.
  • 피드백을 개인적인 지적으로 받아들이지 않습니다. 리뷰 대상은 코드와 코드가 프로덕션 시스템에 미치는 영향입니다.
  • 코드가 무엇을 하는지만이 아니라 왜 존재하는지 설명합니다.
  • 모든 댓글에 답하려고 노력합니다. 완전히 반영한 스레드만 해결 처리합니다. 후속 이슈에서 다룰 수 있는 댓글이라면 진행 방법을 메인테이너와 함께 정합니다.
  • 피드백을 반영한 변경은 별도 커밋으로 푸시합니다. 커밋을 스쿼시하면 리뷰어가 변경을 빠르게 확인하기 어려워집니다.
  • 다음 리뷰를 받을 준비가 되면 리뷰를 다시 요청합니다.
  • 사람 리뷰어에게 리뷰를 요청하기 전에 GitLab Duo 리뷰 댓글을 모두 처리합니다.

작성자 안내: 변경 사항을 더 빨리 병합하는 방법#

  1. 모범 사례를 따릅니다. 설명을 명확하게 작성하고, 스크린샷과 검증 단계를 추가하고, dangerbot 댓글을 처리하고, 승인 체크리스트를 완료합니다.
  2. GitLab 패턴을 따릅니다. 논의가 길어지면 병합이 늦어집니다. 문서화된 방식을 따르고, 모범 사례 변경은 별도 MR로 제안하는 방안을 고려합니다.
  3. MR을 작게 유지합니다. 200줄 정도가 적절한 목표입니다.
    • 작은 MR은 리뷰가 더 빠르고 병합을 막는 논의가 적습니다.
    • 순차적인 MR에는 스택 diff를 사용합니다.
    • MR 마다 메인테이너 한 명만 필요하도록 변경을 나눕니다(예: 기능보다 데이터베이스 변경을 먼저 반영).
    • 모의 데이터를 사용하는 UI는 기능 플래그 뒤에 두어야 합니다.
  4. 리뷰어 수를 최소화합니다. 데이터베이스 리뷰어는 백엔드도 리뷰할 수 있고, 풀스택 엔지니어는 프론트엔드와 백엔드를 모두 담당할 수 있습니다.
  5. 메인테이너를 파악하고 도메인 전문가를 지정합니다. 메인테이너는 자신이 잘 아는 영역의 MR을 우선 처리합니다.

머지 리퀘스트 병합#

병합하기 전에 다음을 진행합니다.

  • 마일스톤을 설정합니다.
  • 올바른 MR 유형 레이블이 적용되었는지 확인합니다.
  • Danger bot, 코드 품질, 그 밖의 리포트에서 나온 경고와 오류를 해결합니다. 실패한 job 이 있는 상태로 병합한다면 댓글을 남깁니다.

병합하기 전에 메인테이너 한 명 이상이 승인해야 합니다. 작성자와 커밋을 추가한 사람은 자신의 MR을 승인할 수 없습니다.

마지막 승인자가 자동 병합을 설정하지 않았다면, 필요한 승인이 모두 있고 병합 권한이 있는 MR 작성자는 자신의 MR을 병합할 수 있습니다. 이는 GitLab의 행동 우선 가치와 부합합니다.

병합할 준비가 되면 다음을 따릅니다.

Warning

머지 리퀘스트가 포크에서 왔다면 커뮤니티 기여 가이드라인도 확인합니다.

  • Squash and merge는 작성자가 설정했거나 커밋 이력이 정리되지 않은 경우에만 사용합니다.
  • Pipelines 탭에서 Run pipeline을 선택한 뒤 Overview 탭에서 Auto-merge를 활성화합니다.
    • 기본 브랜치가 깨진 상태에서는 병합하지 않습니다. 다만 특정 조건에 해당하면 예외입니다.
    • 최신 파이프라인이 승인 전에 생성되었고 MR에 백엔드 변경이 있다면 새 파이프라인을 실행합니다.
    • 최신 병합 결과 파이프라인이 16시간 이내에 생성되었다면 새 파이프라인을 건너뛸 수 있습니다(stable 브랜치는 72시간).

커뮤니티 기여#

Warning

병합 결과 파이프라인을 시작하기 전에 악성 코드가 있는지 모든 변경 사항을 철저히 리뷰합니다.

커뮤니티 MR을 리뷰할 때는 다음을 따릅니다.

  • 새 의존성(예: Gemfile.lock, yarn.lock)을 면밀히 살펴봅니다. 악성 패키지가 들어올 수 있습니다.
  • 특히 문서 MR에서는 링크와 이미지를 확인합니다.
  • 판단이 어렵다면 파이프라인을 시작하기 전에 @gitlab-com/gl-security/appsec에 리뷰를 요청합니다.
  • 마일스톤은 MR 이 현재 마일스톤 안에 병합될 가능성이 있을 때만 설정합니다.

커뮤니티 머지 리퀘스트 인수#

MR에 변경이 필요한데 작성자가 응답하지 않거나 마무리할 수 없는 경우에는 다음을 진행합니다.

  1. 머지 리퀘스트 코치인 본인이 인수한다는 사실을 댓글로 남깁니다.
  2. ~"coach will finish" 레이블을 추가합니다.
  3. main에서 기능 브랜치를 만들고 작성자의 브랜치를 그 브랜치에 병합합니다.
  4. 새 MR을 열고 커뮤니티 MR을 연결한 뒤 ~"Community contribution" 레이블을 추가합니다.
  5. 기여자에게 알리고 일반 리뷰 절차를 따릅니다.

올바른 균형 찾기#

리뷰를 어디까지 깊게 할지 균형을 잡는 데에는 건전한 판단이 필요합니다. 다음을 염두에 둡니다.

  • 버그를 찾는 일은 중요하지만, 좋은 설계는 앞으로의 복잡도를 줄입니다.
  • 코드 스타일은 리뷰 댓글이 아니라 자동화로 강제합니다.
  • non-blocking 제안이라면 MR을 돌려보내기 전에 승인하는 방안을 고려합니다. 병합까지 걸리는 시간이 줄어듭니다.
  • 제대로 하는 것과 지금 당장 하는 것을 구분합니다. 예를 들어 긴급 보안 수정에 대규모 리팩토링을 요구하지 않습니다.
  • 오늘 충분히 잘 해내는 것이 대개 내일 완벽하게 해내는 것보다 낫습니다.

실패하는 파이프라인 문제 해결#

  • 관련 없는 테스트 실패: 같은 실패가 기본 브랜치에서도 발생하는지 확인합니다. 그렇다면 broken master가 수정될 때까지 기다린 뒤 MR의 파이프라인을 다시 실행합니다.
  • danger-review job 실패: MR의 커밋이 20개를 넘는지 확인합니다. 넘는다면 리베이스하고 스쿼시합니다. 그렇지 않으면 job을 다시 실행합니다.

도움이 필요하면 MR에 @gitlab-bot help를 댓글로 남기거나 Community Discord의 contribute 채널에서 문의합니다.

크레딧#

thoughtbot 코드 리뷰 가이드를 기반으로 작성했습니다.

코드 리뷰 가이드라인

GitLab v19.4
원문 보기

요약

GitLab CE와 EE의 모든 머지 리퀘스트는 코드가 효과적이고 이해하기 쉬우며 유지 보수가 가능하고 안전한지 확인하기 위해 코드 리뷰를 거쳐야 합니다. 시작하기 전에 기여 승인 기준을 숙지합니다. 코드는 소속 그룹의 리뷰어 또는 도메인 전문가에게 리뷰를 받습니다.

GitLab CE와 EE의 모든 머지 리퀘스트는 코드가 효과적이고 이해하기 쉬우며 유지 보수가 가능하고 안전한지 확인하기 위해 코드 리뷰를 거쳐야 합니다.

머지 리퀘스트 리뷰, 승인, 병합 받기#

시작하기 전에 기여 승인 기준을 숙지합니다.

코드는 소속 그룹의 리뷰어 또는 도메인 전문가에게 리뷰를 받습니다.

작고 단순한 변경 사항이라면 리뷰어 단계를 건너뛰고 곧바로 메인테이너에게 전달할 수 있습니다. 작고 단순한 변경 사항의 예는 다음과 같습니다.

  • 오타 수정이나 간단한 문구 변경.
  • 동작을 바꾸지 않는 소규모 리팩토링.
  • 한 달 이상 기본값으로 활성화되어 있던 기능 플래그 제거.
  • 사용하지 않는 메서드나 클래스 제거.
  • 다섯 줄 미만의 코드 변경으로 끝나는, 충분히 이해된 로직 변경.

그 밖의 경우에는 MR 이 건드리는 카테고리별로 리뷰어를 지정한 뒤 메인테이너에게 전달합니다. 보안 관련 지원이 필요하면 @gitlab-com/gl-security/appsec을 포함합니다.

리뷰어가 승인하면 메인테이너가 리뷰한 뒤 병합합니다. 마지막 필수 승인자가 병합합니다.

CODEOWNERS가 요구하는 승인은 일반 승인보다 도메인별 승인을 먼저 받습니다. 도메인별 승인자가 메인테이너를 겸하는 경우 두 관점을 함께 리뷰하고 한 번만 승인합니다.

승인 가이드라인#

아래 메인테이너의 책임 절에서 설명하듯이 머지 리퀘스트는 도메인 전문성을 갖춘 메인테이너가 승인하고 병합하도록 하는 것을 권장합니다. 첫 리뷰어의 선택적 승인은 여기서 다루지 않습니다. 다만 머지 리퀘스트는 메인테이너에게 전달하기 전에 개요 절에서 설명한 대로 리뷰어의 리뷰를 받아야 합니다.

머지 리퀘스트에 포함된 내용 필요한 승인자
~backend 변경 사항 1 백엔드 메인테이너.
~database 마이그레이션 또는 비용이 큰 쿼리 변경 사항 2 데이터베이스 메인테이너. 자세한 내용은 데이터베이스 리뷰 가이드라인을 참고합니다.
~workhorse 변경 사항 Workhorse 메인테이너.
~frontend 변경 사항 1 프론트엔드 메인테이너.
~UX 사용자에게 보이는 변경 사항 3 Product Designer. 자세한 내용은 디자인 및 사용자 인터페이스 가이드라인을 참고합니다.
새 JavaScript 라이브러리 추가 1 - 라이브러리가 번들 크기를 크게 늘리는 경우 Frontend Design System 구성원.
- 새 라이브러리가 사용하는 라이선스가 GitLab에서 사용 승인을 받지 않은 경우 법무 부서 구성원.

라이선스 호환성에 대한 자세한 내용은 GitLab 라이선스 및 호환성 문서에서 확인할 수 있습니다.
새 의존성 또는 파일 시스템 변경 - Distribution 팀 구성원. 자세한 내용은 Distribution 팀과 협업하는 방법을 참고합니다.
- RubyGems의 경우 AppSec 리뷰를 요청합니다.
~documentation 또는 ~UI text 변경 사항 해당 DevOps Stage 그룹의 배정에 따른 테크니컬 라이터.
개발 가이드라인 변경 리뷰 프로세스를 따라 그에 맞는 승인을 받습니다.
.ai/ 아래 AI 지시 파일 변경 AI 하네스 DRI. 자세한 내용은 AI 지시 파일 리뷰 가이드라인을 참고합니다.
엔드투엔드 변경과 엔드투엔드 이외 변경 4 Software Engineer in Test.
엔드투엔드 변경만 있는 경우 4 또는 MR 작성자가 Software Engineer in Test 인 경우 Quality 메인테이너.
새로 추가하거나 업데이트한 애플리케이션 제한 프로덕트 매니저.
Analytics Instrumentation(텔레메트리 또는 분석) 변경 사항 Analytics Instrumentation 엔지니어.
GitLab에 추가되는 새 서비스(Puma, Sidekiq, Gitaly 등) 프로덕트 매니저. 자세한 내용은 GitLab에 서비스 컴포넌트를 추가하는 절차를 참고합니다.
인증과 관련된 변경 사항 Manage:Authentication. 자세한 내용은 그룹 페이지의 코드 리뷰 절을 확인합니다. 이 팀의 리뷰가 필요한 것으로 알려진 파일 패턴은 CODEOWNERS 파일의 Authentication 절에 나열되어 있으며, 이 파일을 수정하는 모든 머지 리퀘스트의 승인자 목록에 해당 팀이 표시됩니다.
커스텀 역할 또는 정책과 관련된 변경 사항 Manage:Authorization Engineer.
  1. JavaScript 스펙을 제외한 스펙은 ~backend 코드로 봅니다. Haml 마크업은 ~frontend 코드로 봅니다. 다만 Haml 템플릿 안의 Ruby 코드는 ~backend 코드로 봅니다. 판단이 어려우면 프론트엔드와 백엔드 리뷰를 모두 요청합니다.

    Haml 템플릿 변경의 경우에는 다음과 같이 요청합니다.

    • 템플릿에 Ruby 로직, 메서드 호출, 변수 할당, 조건문, 루프, 데이터 준비, 보안 검사 등 서버 측 처리가 포함된 변경이라면 백엔드 리뷰를 요청합니다.
    • DOM 구조, CSS 클래스, HTML 속성, 접근성 기능, 사용자 상호작용, 반응형 디자인, 시각적 표현에 영향을 주는 변경이라면 프론트엔드 리뷰를 요청합니다.
    • Ruby 로직과 상당한 UI 수정이 함께 들어간 복잡한 변경이거나, 백엔드와 프론트엔드가 서로 얽혀 있는 경우(예: 백엔드가 Vue 나 JavaScript에서 사용하는 데이터를 제공하는 경우)에는 백엔드 기능과 프론트엔드 사용자 경험이 모두 제대로 평가되도록 두 리뷰를 모두 요청합니다.
      • 예: Vue.js 컴포넌트에 전달할 데이터 속성을 준비하기 위해 Ruby 메서드를 호출하는 Haml 템플릿(예: project_id: @project&.to_global_id)이라면 Ruby 로직의 정확성은 백엔드 리뷰가, 컴포넌트 통합은 프론트엔드 리뷰가 도움이 됩니다.
  2. 머지 리퀘스트가 비용이 큰 쿼리를 도입할 가능성이 있으면 데이터베이스 메인테이너의 안내를 받는 것이 좋습니다. 해당 코드 줄에 SQL 쿼리와 함께 댓글을 남기면 조언을 받기에 가장 효율적입니다.

  3. 사용자에게 보이는 변경에는 정도와 무관하게 모든 시각적 변경과, 스크린 리더가 콘텐츠를 읽는 방식에 영향을 주는 렌더링된 DOM 변경이 함께 포함됩니다. 전담 Product Designer가 없는 그룹은 커뮤니티 기여가 아닌 한 기능 변경에 Product Designer의 승인을 받지 않아도 됩니다.

  4. 엔드투엔드 변경에는 qa 디렉터리의 모든 파일이 포함됩니다.

머지 리퀘스트 리뷰#

변경이 필요한 이유(버그 수정, 사용자 경험 개선, 기존 코드 리팩토링)를 먼저 파악합니다. 그다음에는 아래 사항을 따릅니다.

  • 반복 횟수를 줄이기 위해 꼼꼼하게 확인합니다.
  • 강하게 주장하는 의견과 그렇지 않은 의견을 구분해 전달합니다.
  • 문제를 해결하면서도 코드를 단순화할 방법을 찾습니다.
  • 대안 구현을 제시하되 작성자가 이미 그 방법을 검토했다고 가정합니다. ("여기에 커스텀 validator를 쓰는 방안은 어떻습니까?")
  • 작성자의 관점을 이해하려고 노력합니다.
  • 브랜치를 체크아웃해 변경 사항을 로컬에서 테스트합니다. GDK를 크게 수정해야 하는 MR 이라면 대신 스크린샷, 동영상, 도메인 전문가의 검증을 요청하는 방안을 고려합니다. 테스트 과정에서 자동화된 테스트를 추가할 기회를 발견할 수 있습니다.
  • 이해하지 못한 코드가 있으면 그렇다고 말합니다.
  • 의도를 전달할 때는 Conventional Comment 형식을 사용합니다. 필수가 아닌 제안은 (**non-blocking:**)으로 표시합니다. non-blocking 제안만 남았다면 기다리지 않고 MR을 다음 단계로 넘깁니다.
  • 열려 있는 의존성이 없는지 확인합니다. 차단 요소가 있는지 연결된 이슈를 확인합니다. 열려 있는 MR 때문에 막혀 있다면 MR 의존성을 설정합니다.
  • 줄 단위 의견을 남긴 뒤에는 "제가 보기에는 괜찮습니다" 나 "몇 가지만 반영하면 됩니다" 같은 요약 의견을 남깁니다.
  • 리뷰 결과 변경이 필요하면 작성자에게 알립니다.
Warning

머지 리퀘스트가 포크에서 왔다면 커뮤니티 기여 추가 가이드라인도 확인합니다.

GitLab 특정 고려 사항#

GitLab은 Omnibus 패키지부터 소스 설치까지 다양한 환경에서 사용됩니다. GitLab.com 자체도 대규모 Enterprise Edition 인스턴스입니다. 여기에서 다음과 같은 사항이 따라옵니다.

  1. 쿼리 변경은 GitLab.com 규모에서 성능이 나빠지지 않는지 테스트해야 합니다. 데이터베이스 리뷰 가이드라인을 참고합니다.
  2. 데이터베이스 마이그레이션은 다음 조건을 충족해야 합니다.
    1. 되돌릴 수 있어야 합니다.
    2. GitLab.com 규모에서 성능이 충분해야 합니다. 확실하지 않으면 메인테이너에게 스테이징 환경에서 마이그레이션을 테스트해 달라고 요청합니다.
    3. 올바른 마이그레이션 유형이어야 합니다. 어떤 마이그레이션 유형을 고를지는 안내를 참고합니다.
  3. Sidekiq 워커는 하위 호환되지 않는 방식으로 변경할 수 없습니다.
  4. 캐시된 값은 릴리스가 바뀌어도 남아 있을 수 있습니다. 캐시된 값이 반환하는 유형을 바꾸는 경우(예: 문자열이나 nil에서 배열로) 캐시 키도 함께 변경합니다.
  5. 설정은 최후의 수단으로만 추가합니다. GitLab Rails에 새 설정 추가를 참고합니다.
  6. 파일 시스템 액세스는 클라우드 네이티브 아키텍처에서 사용할 수 없습니다. 파일을 저장해야 한다면 오브젝트 스토리지를 지원하는지 확인합니다. 자세한 내용은 업로드 문서를 참고합니다.

머지 리퀘스트 작성자의 책임#

작성자는 최선의 해법을 찾는 직접 책임자 (DRI)입니다. 리뷰 수명 주기 동안 담당자로 남아 있습니다. 스스로를 담당자로 지정할 수 없으면 리뷰어에게 지정을 요청합니다.

너무 크거나 여러 이슈를 함께 수정하거나 복잡도가 높거나 두 개 이상의 기능을 구현하는 머지 리퀘스트는 제출하지 않습니다.

MR 이 여러 CODEOWNERS 절을 건드린다면 필요한 승인을 줄이기 위해 관심사별로 MR을 나눠 리뷰를 병렬로 진행하는 방안을 고려합니다. 메인테이너 리뷰를 요청하기 전에 다음을 확인합니다.

  • MR 이 의도한 문제를 가장 적절한 방식으로 해결하는지.
  • 모든 요구 사항을 충족하는지.
  • 남아 있는 버그, 로직 문제, 다루지 않은 예외 사례, 알려진 취약점이 없는지.

코드 리뷰 가이드라인에 따라 MR을 스스로 리뷰합니다. 결정이나 절충이 있었던 줄, 또는 리뷰어가 코드를 이해하는 데 컨텍스트가 필요한 줄에는 인라인 댓글을 추가합니다.

필요에 따라 도메인 전문가, 프로덕트 매니저, UX 디자이너, 데이터베이스 전문가를 참여시킵니다. MR에 도메인 전문가 리뷰가 필요한지 확실하지 않다면 필요한 것입니다.

MR 이 10개 이상인 기능이라면 EM 이나 Staff Engineer와 함께 컨텍스트를 공유하는 고정 메인테이너를 정합니다.

MR 이 여러 도메인을 건드린다면 도메인마다 전문가의 리뷰를 요청합니다.

리뷰를 요청하기 전에 다음 항목에 대해 MR diff 댓글을 추가합니다.

  • 추가한 린팅 규칙(RuboCop, JS 등).
  • 추가한 라이브러리(Ruby gem, JS 라이브러리 등).
  • 분명하지 않은 상위 클래스나 메서드로 이어지는 링크.
  • 벤치마크 결과.
  • 보안상 문제가 될 수 있는 코드.

리뷰어가 해법을 검증하는 데 필요한 프로젝트, 스니펫, 자산에 접근할 수 있는지 확인합니다.

리뷰어를 지정할 때는 각 리뷰어가 어느 도메인에 집중해야 하는지 댓글로 알립니다. 팀원이 여러 영역에 전문성을 갖춘 경우의 모호함을 없앨 수 있습니다. 예시는 MR 75921 과 MR 109500을 참고합니다.

소스 코드의 TODO 주석은 리뷰어가 요구할 때만 추가합니다. TODO를 추가한다면 관련 이슈 링크를 포함합니다.

주석은 코드가 무엇을 하는지만이 아니라 왜 그런지 설명하도록 작성합니다.

메인테이너 리뷰는 테스트가 통과한 뒤에만 요청합니다. 테스트가 실패했다면 그 이유를 댓글로 설명합니다. 메인테이너에게 이메일이나 Slack으로 연락하는 것은 즉시 처리해야 하는 요청에만 한정합니다. 그 밖의 경우에는 리뷰어로 추가하는 것으로 충분합니다.

리뷰어의 책임#

리뷰어는 선택된 해법의 세부 내용을 리뷰할 책임이 있습니다.

리뷰 응답 SLO 안에 리뷰할 수 없다면 작성자에게 알리고, 리뷰 업무량 대시보드에서 대체 리뷰어를 찾아 지정합니다.

MR 이 모든 기여 승인 기준을 충족한다고 확신하면 다음을 진행합니다.

  1. Approve를 선택합니다.
  2. 작성자에게 알리기 위해 @로 멘션합니다.
  3. 도메인 전문성을 갖춘 메인테이너에게 리뷰를 요청하거나 리뷰어 룰렛의 추천을 따릅니다.

메인테이너의 책임#

메인테이너는 GitLab 코드베이스 전반의 건전성, 품질, 일관성에 대한 책임을 집니다. 리뷰에서는 아키텍처, 코드 구성, 관심사 분리, 테스트, DRY 여부, 일관성, 가독성에 초점을 맞춥니다.

메인테이너는 MR 이 승인 기준을 합리적으로 충족하는지 확인하는 DRI 입니다.

메인테이너는 MR의 영향을 평가할 때 건전한 판단을 내립니다. MR을 병합할 수 없다고 판단하면 그 사실을 말하는 것도 메인테이너의 책임입니다. 또한 메인테이너는 다른 사람의 의견을 들어야 할 시점을 아는 전문 조언자 역할도 합니다.

메인테이너가 MR을 승인하면 작성자와 함께 책임을 지게 됩니다. 따라서 프로덕션 사고가 발생하면 문제 해결을 돕기 위해 메인테이너가 호출될 수 있습니다.

일부 머지 리퀘스트는 stable 브랜치를 대상으로 합니다. 이런 요청을 처리하는 방법은 패치 릴리스 런북에서 확인합니다.

모범 사례#

도메인 전문가#

도메인 전문가는 특정 기술, 제품 기능, 코드베이스 영역에 상당한 경험을 갖춘 팀원입니다. 팀원은 스스로 도메인 전문가임을 밝히고 이를 팀 프로필에 추가하는 것이 좋습니다.

스스로 도메인 전문가임을 밝힐 때는 .yml 파일을 변경하는 MR을 이미 인정받은 도메인 전문가나 담당 Engineering Manager가 병합하도록 지정하는 것이 좋습니다.

자동으로 도메인 전문가로 간주되는 경우에 대해서는 다음과 같이 가정합니다.

  • 특정 스테이지/그룹(예: create: source code)에서 일하는 팀원은 자신이 담당하는 애플리케이션 영역의 도메인 전문가로 간주합니다.
  • 특정 기능(예: search)을 담당하는 팀원은 그 기능의 도메인 전문가로 간주합니다.

코드 리뷰는 기본적으로 도메인 전문성을 갖춘 팀원에게 배정합니다. UX 리뷰는 기본적으로 Review Roulette가 추천한 리뷰어에게 배정합니다. 디자이너 가용 인력 제한으로 Product Designer가 지원하지 않는 영역은 커뮤니티 기여가 아닌 한 UX 리뷰를 요구하지 않습니다. 적합한 도메인 전문가가 없으면 다른 팀원에게 MR 리뷰를 요청하거나 리뷰어 룰렛의 추천을 따를 수 있습니다(UX 리뷰는 위 내용을 참고합니다). 지정하기 전에 해당 팀원이 부재 중인지 다시 확인합니다.

도메인 전문가를 찾는 방법은 다음과 같습니다.

  • Merge Request 승인 위젯에서 View eligible approvers를 선택합니다. 이 위젯은 코드베이스 영역별로 권장 승인과 필수 승인을 보여 줍니다. 이 규칙은 Code Owners에 정의되어 있습니다.
  • 머지 리퀘스트와 관련된 스테이지 또는 그룹에서 일하는 팀원 목록을 확인합니다.
  • engineering projects 페이지나 GitLab 팀 페이지에서 팀원의 도메인 전문성을 확인합니다. 도메인은 본인이 밝힌 것이므로, 머지 리퀘스트의 변경 사항을 도메인에 연결할 때는 스스로 판단합니다.
  • 머지 리퀘스트의 파일에 기여한 팀원을 찾습니다. git log <file>을 실행해 로그를 확인합니다.
  • 해당 파일을 리뷰한 팀원을 찾습니다. 관련 머지 리퀘스트는 다음 방법으로 찾습니다.
    1. git log <file>로 커밋 SHA를 확인합니다.
    2. https://gitlab.com/gitlab-org/gitlab/-/commit/로 이동합니다.
    3. 커밋에 표시된 관련 머지 리퀘스트를 선택합니다.

리뷰어 룰렛#

Note

리뷰어 룰렛은 GitLab.com을 위한 내부 도구이며 고객 설치 환경에서는 사용할 수 없습니다.

Danger bot은 MR 이 건드리는 코드베이스 영역마다 리뷰어와 메인테이너를 고릅니다. 더 적합한 사람을 알고 있다면 추천을 대신해 직접 지정합니다.

GitLab은 룰렛 추천 표의 Reviewer 칼럼이 여전히 유용한지 평가하는 실험을 진행하고 있습니다. CI/CD 변수로 제어되는 일부 머지 리퀘스트에서는 Danger가 Reviewer 칼럼을 숨기고 Category와 Maintainer 칼럼만 표시합니다. 해당되는 MR 이라도 위에 링크된 리뷰 업무량 대시보드로 리뷰어를 찾을 수 있습니다. 실험에 포함된 MR에는 결과를 측정할 수 있도록 roulette-experiment::reviewer-column-hidden 또는 roulette-experiment::reviewer-column-shown 레이블이 지정됩니다.

룰렛은 상태에 OOO, PTO, Parental Leave, Friends and Family, Conference가 포함된 사람과 리뷰 가능 한도에 도달한 사람(숫자 상태 이모지 2️⃣-5️⃣ 로 설정)을 건너뜁니다.

승인 체크리스트#

이 체크리스트는 머지 리퀘스트(MR)의 작성자, 리뷰어, 메인테이너가 품질, 성능, 신뢰성, 보안, 가관측성, 유지 보수성에 큰 영향을 주는 위험 요소를 분석했는지 확인하도록 돕습니다.

체크리스트를 사용하면 소프트웨어 엔지니어링의 품질이 높아집니다. 이 체크리스트는 GitLab 코드베이스 기여자의 역량을 뒷받침하는 간단한 도구입니다.

품질#

품질 가이드라인에 대한 자세한 내용은 테스팅을 참고합니다.

  1. 코드 리뷰 가이드라인에 따라 이 MR을 스스로 리뷰했습니다.
  2. 코드가 소프트웨어 설계 가이드라인을 따릅니다.
  3. 테스트 피라미드에 따라 자동화된 테스트가 있는지 확인합니다. 빠진 테스트를 추가하거나 테스트 공백을 기록한 이슈를 만듭니다.
  4. GitLab.com, Dedicated, self-managed에 미치는 기술적 영향을 검토했습니다.
  5. 이 변경이 시스템의 프론트엔드, 백엔드, 데이터베이스 부분에 미치는 영향을 적절히 검토하고 ~ux, ~frontend, ~backend, ~database 레이블을 그에 맞게 적용했습니다.
  6. 지원되는 모든 브라우저에서 이 MR을 테스트했거나, 이 테스트가 필요하지 않다고 판단했습니다.
  7. 이 변경이 업데이트 간 하위 호환성을 유지하는지 확인했거나, 해당하지 않는다고 판단했습니다.
  8. EE 콘텐츠가 있다면 FOSS와 올바르게 분리했습니다. FOSS 컨텍스트에서 CI 파이프라인 실행을 고려합니다.
  9. 기존 데이터가 예상보다 다양할 수 있다는 점을 검토했습니다. 예를 들어 새 모델 유효성 검사를 추가한다면 기존 데이터에는 선택 적용하는 방안을 고려합니다.
  10. 이 MR과 관련된 불안정한 테스트를 수정했거나, 무시해도 되는 이유를 설명했습니다. 불안정한 테스트에는 Flaky test '<path/to/test>' was found in the list of files changed by this MR. 오류가 있지만, 경고와 함께 통과하는 job에 있을 수도 있습니다.

성능, 신뢰성 및 가용성#

  1. 이 MR 이 성능을 해치지 않는다고 확신하거나, 성능 영향 평가를 리뷰어에게 요청했습니다. (머지 리퀘스트 성능 가이드라인)
  2. MR 설명에 데이터베이스 리뷰어를 위한 정보를 추가했거나, 필요하지 않다고 판단했습니다.
  3. 이 변경의 가용성 및 신뢰성 위험을 검토했습니다.
  4. 향후 예상 성장에 따른 확장성 위험을 검토했습니다.
  5. 평균적인 고객보다 데이터가 훨씬 많을 수 있는 대규모 고객에 이 변경이 미치는 성능, 신뢰성, 가용성 영향을 검토했습니다.
  6. 최소 시스템 요건에서 GitLab을 운영하는 고객에 이 변경이 미치는 성능, 신뢰성, 가용성 영향을 검토했습니다.
  7. 이 변경이 Cells 아키텍처와 호환된다고 확신합니다. 자세한 내용은 Cells 개발 원칙을 참고합니다.

가관측성 계측#

  1. 가관측성을 통해 디버깅과 선제적 성능 개선이 가능하도록 충분한 계측을 포함했습니다. 기능 플래그, 로깅, 계측을 추가한 예시를 참고합니다.

문서#

  1. changelog 트레일러를 포함했거나, 필요하지 않다고 판단했습니다.
  2. 문서를 추가하거나 업데이트했거나, 이 MR에는 문서 변경이 필요하지 않다고 판단했습니다.

보안#

  1. 이 MR에 자격 증명이나 토큰의 처리·저장, 인가 및 인증 메서드, 그 밖에 보안 리뷰 가이드라인에 설명된 항목의 변경이 포함된다면 ~security 레이블을 추가하고 @gitlab-com/gl-security/appsec을 @로 멘션했습니다.
  2. 보안 리뷰를 언제 어떻게 요청하는지에 대해 내부 애플리케이션 보안 리뷰 문서를 확인했고, 이 변경에 보안 리뷰가 필요하다고 판단해 리뷰를 요청했습니다.
  3. 머지 리퀘스트 승인 정책으로 인해 MR을 막고 있는 보안 스캔 결과가 있는 경우:
    • 참 양성 결과는 머지 리퀘스트를 병합하기 전에 수정합니다. 수정하면 머지 리퀘스트 승인 정책이 요구하는 AppSec 승인이 해제됩니다.
    • 거짓 양성 결과, 위험 수용을 논의해야 하는 사항, 판단이 애매한 사항은 @gitlab-com/gl-security/appsec에 문의합니다.

배포#

  1. 이 변경이 위험도가 높을 수 있으므로 기능 플래그 사용을 검토했습니다.
  2. 기능 플래그를 사용한다면 프로덕션에서 테스트하기 전에 스테이징에서 변경을 테스트할 계획이며, 모든 고객에게 배포하기 전에 일부 프로덕션 고객에게 먼저 배포하는 방안을 검토했습니다.
  3. 완료의 정의에 따라 기본 설정 또는 새 설정 변경을 Infrastructure 부서에 알렸거나, 필요하지 않다고 판단했습니다.

규정 준수#

  1. 올바른 MR 유형 레이블이 적용되었는지 확인했습니다.

코드 리뷰 참여#

  • 친절하게 대합니다.
  • 프로그래밍 결정의 상당수가 의견임을 받아들입니다. 절충점을 논의하고 빠르게 정리합니다.
  • 질문을 합니다. 요구가 아니라 제안을 합니다.
  • 명확하게 표현합니다. 온라인에서는 의도가 늘 전달되지는 않습니다.
  • 겸허한 태도를 유지합니다. 오해가 길어지면 일대일 통화를 고려하고 이후 요약을 남깁니다.
  • 특정 사람에게 하는 댓글이라면 그 사람을 직접 멘션합니다.
  • 처음 푸시하기 전에 전체 diff를 읽습니다. 관련 없는 변경과 디버그 코드가 있는지 확인합니다.
  • 머지 리퀘스트 가이드라인에 따라 상세한 설명을 작성합니다.
  • 피드백을 개인적인 지적으로 받아들이지 않습니다. 리뷰 대상은 코드와 코드가 프로덕션 시스템에 미치는 영향입니다.
  • 코드가 무엇을 하는지만이 아니라 왜 존재하는지 설명합니다.
  • 모든 댓글에 답하려고 노력합니다. 완전히 반영한 스레드만 해결 처리합니다. 후속 이슈에서 다룰 수 있는 댓글이라면 진행 방법을 메인테이너와 함께 정합니다.
  • 피드백을 반영한 변경은 별도 커밋으로 푸시합니다. 커밋을 스쿼시하면 리뷰어가 변경을 빠르게 확인하기 어려워집니다.
  • 다음 리뷰를 받을 준비가 되면 리뷰를 다시 요청합니다.
  • 사람 리뷰어에게 리뷰를 요청하기 전에 GitLab Duo 리뷰 댓글을 모두 처리합니다.

작성자 안내: 변경 사항을 더 빨리 병합하는 방법#

  1. 모범 사례를 따릅니다. 설명을 명확하게 작성하고, 스크린샷과 검증 단계를 추가하고, dangerbot 댓글을 처리하고, 승인 체크리스트를 완료합니다.
  2. GitLab 패턴을 따릅니다. 논의가 길어지면 병합이 늦어집니다. 문서화된 방식을 따르고, 모범 사례 변경은 별도 MR로 제안하는 방안을 고려합니다.
  3. MR을 작게 유지합니다. 200줄 정도가 적절한 목표입니다.
    • 작은 MR은 리뷰가 더 빠르고 병합을 막는 논의가 적습니다.
    • 순차적인 MR에는 스택 diff를 사용합니다.
    • MR 마다 메인테이너 한 명만 필요하도록 변경을 나눕니다(예: 기능보다 데이터베이스 변경을 먼저 반영).
    • 모의 데이터를 사용하는 UI는 기능 플래그 뒤에 두어야 합니다.
  4. 리뷰어 수를 최소화합니다. 데이터베이스 리뷰어는 백엔드도 리뷰할 수 있고, 풀스택 엔지니어는 프론트엔드와 백엔드를 모두 담당할 수 있습니다.
  5. 메인테이너를 파악하고 도메인 전문가를 지정합니다. 메인테이너는 자신이 잘 아는 영역의 MR을 우선 처리합니다.

머지 리퀘스트 병합#

병합하기 전에 다음을 진행합니다.

  • 마일스톤을 설정합니다.
  • 올바른 MR 유형 레이블이 적용되었는지 확인합니다.
  • Danger bot, 코드 품질, 그 밖의 리포트에서 나온 경고와 오류를 해결합니다. 실패한 job 이 있는 상태로 병합한다면 댓글을 남깁니다.

병합하기 전에 메인테이너 한 명 이상이 승인해야 합니다. 작성자와 커밋을 추가한 사람은 자신의 MR을 승인할 수 없습니다.

마지막 승인자가 자동 병합을 설정하지 않았다면, 필요한 승인이 모두 있고 병합 권한이 있는 MR 작성자는 자신의 MR을 병합할 수 있습니다. 이는 GitLab의 행동 우선 가치와 부합합니다.

병합할 준비가 되면 다음을 따릅니다.

Warning

머지 리퀘스트가 포크에서 왔다면 커뮤니티 기여 가이드라인도 확인합니다.

  • Squash and merge는 작성자가 설정했거나 커밋 이력이 정리되지 않은 경우에만 사용합니다.
  • Pipelines 탭에서 Run pipeline을 선택한 뒤 Overview 탭에서 Auto-merge를 활성화합니다.
    • 기본 브랜치가 깨진 상태에서는 병합하지 않습니다. 다만 특정 조건에 해당하면 예외입니다.
    • 최신 파이프라인이 승인 전에 생성되었고 MR에 백엔드 변경이 있다면 새 파이프라인을 실행합니다.
    • 최신 병합 결과 파이프라인이 16시간 이내에 생성되었다면 새 파이프라인을 건너뛸 수 있습니다(stable 브랜치는 72시간).

커뮤니티 기여#

Warning

병합 결과 파이프라인을 시작하기 전에 악성 코드가 있는지 모든 변경 사항을 철저히 리뷰합니다.

커뮤니티 MR을 리뷰할 때는 다음을 따릅니다.

  • 새 의존성(예: Gemfile.lock, yarn.lock)을 면밀히 살펴봅니다. 악성 패키지가 들어올 수 있습니다.
  • 특히 문서 MR에서는 링크와 이미지를 확인합니다.
  • 판단이 어렵다면 파이프라인을 시작하기 전에 @gitlab-com/gl-security/appsec에 리뷰를 요청합니다.
  • 마일스톤은 MR 이 현재 마일스톤 안에 병합될 가능성이 있을 때만 설정합니다.

커뮤니티 머지 리퀘스트 인수#

MR에 변경이 필요한데 작성자가 응답하지 않거나 마무리할 수 없는 경우에는 다음을 진행합니다.

  1. 머지 리퀘스트 코치인 본인이 인수한다는 사실을 댓글로 남깁니다.
  2. ~"coach will finish" 레이블을 추가합니다.
  3. main에서 기능 브랜치를 만들고 작성자의 브랜치를 그 브랜치에 병합합니다.
  4. 새 MR을 열고 커뮤니티 MR을 연결한 뒤 ~"Community contribution" 레이블을 추가합니다.
  5. 기여자에게 알리고 일반 리뷰 절차를 따릅니다.

올바른 균형 찾기#

리뷰를 어디까지 깊게 할지 균형을 잡는 데에는 건전한 판단이 필요합니다. 다음을 염두에 둡니다.

  • 버그를 찾는 일은 중요하지만, 좋은 설계는 앞으로의 복잡도를 줄입니다.
  • 코드 스타일은 리뷰 댓글이 아니라 자동화로 강제합니다.
  • non-blocking 제안이라면 MR을 돌려보내기 전에 승인하는 방안을 고려합니다. 병합까지 걸리는 시간이 줄어듭니다.
  • 제대로 하는 것과 지금 당장 하는 것을 구분합니다. 예를 들어 긴급 보안 수정에 대규모 리팩토링을 요구하지 않습니다.
  • 오늘 충분히 잘 해내는 것이 대개 내일 완벽하게 해내는 것보다 낫습니다.

실패하는 파이프라인 문제 해결#

  • 관련 없는 테스트 실패: 같은 실패가 기본 브랜치에서도 발생하는지 확인합니다. 그렇다면 broken master가 수정될 때까지 기다린 뒤 MR의 파이프라인을 다시 실행합니다.
  • danger-review job 실패: MR의 커밋이 20개를 넘는지 확인합니다. 넘는다면 리베이스하고 스쿼시합니다. 그렇지 않으면 job을 다시 실행합니다.

도움이 필요하면 MR에 @gitlab-bot help를 댓글로 남기거나 Community Discord의 contribute 채널에서 문의합니다.

크레딧#

thoughtbot 코드 리뷰 가이드를 기반으로 작성했습니다.