InfoGrab DocsInfoGrab Docs

코드 리뷰 가이드라인

요약

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

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

머지 리퀘스트 검토, 승인 및 병합 받기#

시작하기 전에 기여 승인 기준을 숙지하세요.

그룹 소속 리뷰어 또는 도메인 전문가에게 코드 검토를 받으세요.

작고 단순한 변경 사항의 경우 리뷰어 단계를 건너뛰고 메인테이너에게 직접 요청할 수 있습니다. 작고 단순한 변경의 예시:

  • 오탈자 수정 또는 소규모 텍스트 변경.

  • 동작을 변경하지 않는 소규모 리팩토링.

  • 한 달 이상 기본 활성화된 기능 플래그 제거.

  • 사용하지 않는 메서드나 클래스 제거.

  • 5줄 미만의 코드 변경이 필요한, 잘 이해된 로직 변경.

그 외의 경우, 메인테이너에게 전달하기 전에 MR이 닿는 각 카테고리의 리뷰어 승인을 받으세요. 보안 지원이 필요한 경우 @gitlab-com/gl-security/appsec를 포함하세요.

리뷰어가 승인한 후 메인테이너가 검토하고 병합합니다. 마지막으로 필요한 승인자가 병합합니다.

CODEOWNERS 필수 승인의 경우, 일반 승인보다 도메인별 승인을 먼저 구하세요. 메인테이너이기도 한 도메인별 승인자는 두 측면을 모두 검토하고 한 번만 승인해야 합니다.

승인 가이드라인#

아래 메인테이너 책임 섹션에서 설명하는 바와 같이, 도메인 전문성을 보유한 메인테이너에게 머지 리퀘스트 승인 및 병합을 받도록 권장합니다. 첫 번째 리뷰어의 선택적 승인은 여기서 다루지 않습니다. 그러나 머지 리퀘스트는 개요 섹션에서 설명한 대로 메인테이너에게 전달하기 전에 리뷰어의 검토를 받아야 합니다.

머지 리퀘스트에 포함된 내용 필요한 승인자
~backend 변경 사항 1 백엔드 메인테이너.
~database 마이그레이션 또는 비용이 많이 드는 쿼리 변경 사항 2 데이터베이스 메인테이너. 자세한 내용은 데이터베이스 리뷰 가이드라인을 참조하세요.
~workhorse 변경 사항 Workhorse 메인테이너.
~frontend 변경 사항 1 프론트엔드 메인테이너.
~UX 사용자 대면 변경 사항 3 프로덕트 디자이너. 자세한 내용은 디자인 및 사용자 인터페이스 가이드라인을 참조하세요.
새로운 JavaScript 라이브러리 추가 1 - 라이브러리가 번들 크기를 크게 증가시키는 경우 프론트엔드 디자인 시스템 멤버.- 새 라이브러리에서 사용하는 라이선스가 GitLab 사용에 승인되지 않은 경우 법무 부서 멤버. 라이선스 호환성에 대한 자세한 정보는 GitLab 라이선스 및 호환성 문서를 참조하세요.
새로운 의존성 또는 파일 시스템 변경 - Distribution 팀 멤버. 자세한 내용은 Distribution 팀과의 협력 방법을 참조하세요.- RubyGems의 경우 AppSec 리뷰를 요청하세요.
~documentation 또는 ~UI 텍스트 변경 해당 DevOps Stage 그룹 담당 기술 작성자.
개발 가이드라인 변경 검토 프로세스에 따라 해당 승인을 받으세요.
엔드투엔드 및 비엔드투엔드 변경 사항 4 테스트 소프트웨어 엔지니어.
엔드투엔드 변경 사항만 4 또는 MR 작성자가 테스트 소프트웨어 엔지니어인 경우 Quality 메인테이너.
새로운 또는 업데이트된 애플리케이션 제한 프로덕트 매니저.
Analytics Instrumentation(원격 측정 또는 분석) 변경 사항 Analytics Instrumentation 엔지니어.
GitLab에 새로운 서비스 추가(Puma, Sidekiq, Gitaly 등) 프로덕트 매니저. 자세한 내용은 GitLab에 서비스 컴포넌트 추가 프로세스를 참조하세요.
인증 관련 변경 사항 Manage:Authentication. 자세한 내용은 그룹 페이지의 코드 리뷰 섹션을 확인하세요. 팀 검토가 필요한 것으로 알려진 파일 패턴은 CODEOWNERS 파일의 Authentication 섹션에 나열되어 있으며, 해당 파일을 수정하는 모든 머지 리퀘스트의 승인자 섹션에 팀이 포함됩니다.
커스텀 역할 또는 정책 관련 변경 사항 Manage:Authorization 엔지니어.

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 로직 정확성을 위한 백엔드 검토와 컴포넌트 통합을 위한 프론트엔드 검토를 모두 받으면 좋습니다.

머지 리퀘스트가 잠재적으로 비용이 많이 드는 쿼리를 도입하는 경우 데이터베이스 메인테이너에게 지도를 구하도록 권장합니다. SQL 쿼리와 함께 해당 코드 라인에 댓글을 달면 조언을 받기 가장 효율적입니다.

사용자 대면 변경 사항에는 시각적 변경(아무리 사소하더라도)과 스크린 리더가 콘텐츠를 안내하는 방식에 영향을 미치는 렌더링된 DOM 변경이 모두 포함됩니다. 전담 프로덕트 디자이너가 없는 그룹은 커뮤니티 기여가 아닌 한 기능 변경에 대해 프로덕트 디자이너의 승인이 필요하지 않습니다.

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

머지 리퀘스트 검토#

변경이 필요한 이유(버그 수정, 사용자 경험 개선, 기존 코드 리팩토링)를 이해하세요. 그런 다음:

  • 반복 횟수를 줄이기 위해 철저하게 검토하세요.

  • 강하게 생각하는 아이디어와 그렇지 않은 아이디어를 구분해서 소통하세요.

  • 문제를 해결하면서도 코드를 단순화할 방법을 찾으세요.

  • 대안 구현을 제안하되, 작성자가 이미 고려했다고 가정하세요. ("여기서 커스텀 유효성 검사기를 사용하는 것은 어떻게 생각하세요?")

  • 작성자의 관점을 이해하려고 노력하세요.

  • 브랜치를 체크아웃하고 변경 사항을 로컬에서 테스트하세요. 상당한 GDK 수정이 필요한 MR의 경우, 스크린샷, 동영상 또는 도메인 전문가 검증을 요청하는 것을 고려하세요. 테스트를 통해 자동화된 테스트를 추가할 기회를 얻을 수 있습니다.

  • 코드를 이해하지 못하면 그렇다고 말하세요.

  • Conventional Comment 형식을 사용하여 의도를 전달하세요. 비필수 제안은 (**non-blocking:**)으로 표시하세요. 비차단 제안만 남은 경우, 기다리지 말고 MR을 다음 단계로 이동하세요.

  • 열린 의존성이 없는지 확인하세요. 차단 요소가 있는지 연결된 이슈를 확인하세요. 열린 MR에 의해 차단된 경우 MR 의존성을 설정하세요.

  • 라인 노트 한 라운드 후 "괜찮아 보입니다" 또는 "몇 가지 처리할 사항이 있습니다" 같은 요약 노트를 작성하세요.

  • 검토 후 변경이 필요한 경우 작성자에게 알려주세요.

    머지 리퀘스트가 포크에서 온 경우 커뮤니티 기여에 대한 추가 가이드라인도 확인하세요.

GitLab 특정 고려 사항#

GitLab은 Omnibus 패키지부터 소스 설치까지 많은 곳에서 사용됩니다. GitLab.com 자체는 대규모 Enterprise Edition 인스턴스입니다. 이에 따른 몇 가지 시사점이 있습니다:

  • 쿼리 변경 사항은 GitLab.com 규모에서 성능 저하가 발생하지 않는지 테스트해야 합니다. 데이터베이스 리뷰 가이드라인을 참조하세요.

  • 데이터베이스 마이그레이션은 다음을 충족해야 합니다:

되돌릴 수 있어야 합니다.

  • GitLab.com 규모에서 성능이 우수해야 합니다. 확실하지 않으면 메인테이너에게 스테이징 환경에서 마이그레이션을 테스트해 달라고 요청하세요.

  • 올바른 마이그레이션 유형이어야 합니다. 어떤 마이그레이션 유형을 선택해야 하는지에 대한 지침을 참조하세요.

  • Sidekiq 워커는 하위 호환되지 않는 방식으로 변경할 수 없습니다.

  • 캐시된 값은 릴리스 전반에 걸쳐 지속될 수 있습니다. 캐시된 값이 반환하는 유형을 변경하는 경우(예: 문자열 또는 nil에서 배열로), 동시에 캐시 키를 변경하세요.

  • 설정은 최후의 수단으로 추가해야 합니다. GitLab Rails에 새 설정 추가를 참조하세요.

  • 파일 시스템 접근은 클라우드 네이티브 아키텍처에서는 불가능합니다. 수행해야 하는 모든 파일 스토리지에 대해 오브젝트 스토리지를 지원하는지 확인하세요. 자세한 내용은 업로드 문서를 참조하세요.

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

최선의 솔루션을 찾는 직접 책임 개인(DRI)은 바로 여러분입니다. 검토 라이프사이클 전반에 걸쳐 담당자로 남아 있으세요. 스스로 담당자로 설정할 수 없는 경우, 리뷰어에게 요청하세요.

너무 크거나, 여러 이슈를 수정하거나, 복잡성이 높거나, 둘 이상의 기능을 구현하는 머지 리퀘스트를 제출하지 마세요.

MR이 여러 CODEOWNERS 섹션에 걸쳐 있는 경우, 필요한 승인 수를 최소화하기 위해 관심사별로 MR을 분리하여 검토를 병렬화하는 것을 고려하세요. 메인테이너 검토를 요청하기 전에 다음을 확인하세요:

  • MR이 가장 적절한 방식으로 의도한 문제를 해결하는지.

  • 모든 요구 사항이 충족되었는지.

  • 남은 버그, 로직 문제, 처리되지 않은 엣지 케이스 또는 알려진 취약점이 없는지.

코드 리뷰 가이드라인에 따라 MR을 셀프 리뷰하세요. 결정이나 트레이드오프를 한 라인이나 리뷰어가 코드를 이해하는 데 컨텍스트가 도움이 되는 라인에 인라인 댓글을 추가하세요.

적절한 경우 도메인 전문가, 프로덕트 매니저, UX 디자이너, 데이터베이스 전문가를 참여시키세요. MR에 도메인 전문가 검토가 필요한지 확실하지 않으면 필요한 것입니다.

10개 이상의 MR에 걸친 기능의 경우 EM 또는 Staff 엔지니어와 협력하여 컨텍스트를 공유하는 일관된 메인테이너를 식별하세요.

MR이 여러 도메인에 걸쳐 있는 경우, 각 도메인 전문가에게 검토를 요청하세요.

검토를 요청하기 전에 다음에 대한 MR diff 댓글을 추가하세요:

  • 추가된 린팅 규칙(RuboCop, JS 등).

  • 추가된 라이브러리(Ruby 젬, JS 라이브러리 등).

  • 명확하지 않은 경우 부모 클래스나 메서드에 대한 링크.

  • 벤치마킹 결과.

  • 잠재적으로 보안에 취약한 코드.

리뷰어가 솔루션을 검증하는 데 필요한 프로젝트, 스니펫 또는 자산에 접근할 수 있는지 확인하세요.

리뷰어를 할당할 때 각 리뷰어가 집중해야 할 도메인을 명시하는 댓글을 남기세요. 이는 팀 멤버가 여러 영역에 전문 지식을 가지고 있을 때 모호함을 방지합니다. 예시는 MR 75921과 MR 109500을 참조하세요.

리뷰어가 요구하는 경우에만 소스 코드에 TODO 댓글을 추가하세요. TODO를 추가하는 경우 관련 이슈에 대한 링크를 포함하세요.

왜, 무엇을만이 아닌 코드가 하는 일을 설명하는 댓글을 작성하세요.

테스트가 통과할 때만 메인테이너 검토를 요청하세요. 테스트가 실패하는 경우 댓글에 이유를 설명하세요. 즉각적인 요청의 경우에만 이메일이나 Slack으로 메인테이너에게 연락하세요. 그 외의 경우 리뷰어로 추가하는 것으로 충분합니다.

리뷰어의 책임#

리뷰어는 선택된 솔루션의 세부 사항을 검토할 책임이 있습니다.

검토 응답 SLO 내에 응답이 불가능한 경우, 작성자에게 알리고 검토 워크로드 대시보드를 사용하여 대체자를 찾아 할당하세요.

MR이 모든 기여 승인 기준을 충족한다고 확신하는 경우:

  • 승인을 선택하세요.

  • 작성자에게 알리기 위해 @ 멘션하세요.

  • 도메인 전문성을 가진 메인테이너에게 검토를 요청하거나 리뷰어 룰렛 제안을 따르세요.

메인테이너의 책임#

메인테이너는 GitLab 코드베이스의 전반적인 건강, 품질 및 일관성에 대한 책임이 있습니다. 검토는 아키텍처, 코드 구성, 관심사 분리, 테스트, DRYness, 일관성 및 가독성에 초점을 맞춥니다.

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

메인테이너는 MR의 영향을 평가할 때 합리적인 판단을 내립니다. 메인테이너가 MR을 병합할 수 없다고 판단하면 그렇게 말하는 것이 그들의 책임입니다. 메인테이너는 또한 두 번째 의견을 위해 다른 사람을 참여시켜야 할 때를 아는 전문 자문가입니다.

메인테이너가 MR을 승인하면 작성자와 함께 책임을 지게 됩니다. 이는 프로덕션 사고가 발생했을 때 메인테이너가 문제 해결을 위해 호출될 수 있음을 의미합니다.

특정 머지 리퀘스트는 안정 브랜치를 대상으로 할 수 있습니다. 이러한 요청을 처리하는 방법에 대한 개요는 패치 릴리즈 런북을 참조하세요.

모범 사례#

도메인 전문가#

도메인 전문가는 특정 기술, 제품 기능 또는 코드베이스 영역에 상당한 경험을 가진 팀 멤버입니다. 팀 멤버는 자신을 도메인 전문가로 자가 식별하고 팀 프로필에 추가하도록 권장됩니다.

도메인 전문가로 자가 식별할 때는 .yml 파일을 변경하는 MR을 이미 확립된 도메인 전문가 또는 해당 엔지니어링 매니저가 병합하도록 할당하는 것이 권장됩니다.

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

  • 특정 Stage/그룹(예: create: source code)에서 일하는 팀 멤버는 그들이 작업하는 앱 영역의 도메인 전문가로 간주됩니다.

  • 특정 기능(예: 검색)에서 일하는 팀 멤버는 해당 기능의 도메인 전문가로 간주됩니다.

코드 리뷰의 경우 기본적으로 도메인 전문성을 가진 팀 멤버에게 검토를 할당합니다. UX 검토는 기본적으로 검토 룰렛에서 추천된 리뷰어로 합니다. 디자이너 용량 제한으로 인해 프로덕트 디자이너가 지원하지 않는 영역은 커뮤니티 기여가 아닌 한 더 이상 UX 검토가 필요하지 않습니다. 적합한 도메인 전문가를 구할 수 없는 경우 어떤 팀 멤버든 MR을 검토하도록 선택하거나 리뷰어 룰렛 추천을 따를 수 있습니다(UX 검토에 대해서는 위를 참조). 할당하기 전에 해당 인물이 부재중인지 확인하세요.

도메인 전문가를 찾으려면:

  • 머지 리퀘스트 승인 위젯에서 적격 승인자 보기를 선택하세요. 이 위젯은 코드베이스의 각 영역별 권장 및 필수 승인을 보여줍니다. 이러한 규칙은 Code Owners에 정의되어 있습니다.

  • 머지 리퀘스트와 관련된 Stage 또는 그룹에서 일하는 팀 멤버 목록을 확인하세요.

  • 엔지니어링 프로젝트 페이지 또는 GitLab 팀 페이지에서 팀 멤버의 도메인 전문성을 확인하세요. 도메인은 자가 식별되므로 머지 리퀘스트의 변경 사항을 도메인에 매핑할 때 판단을 활용하세요.

  • 머지 리퀘스트의 파일에 기여한 팀 멤버를 찾으세요. git log <file>을 실행하여 로그를 확인하세요.

  • 파일을 검토한 팀 멤버를 찾으세요. 관련 머지 리퀘스트를 다음 방법으로 찾을 수 있습니다:

git log <file>을 사용하여 커밋 SHA를 가져오세요.

  • https://gitlab.com/gitlab-org/gitlab/-/commit/로 이동하세요.

  • 커밋에 대해 표시된 관련 머지 리퀘스트를 선택하세요.

리뷰어 룰렛#

[리뷰어 룰렛](https://gitlab-org.gitlab.io/gitlab-roulette/)은 GitLab.com용 내부 도구로, 고객 설치에서는 사용할 수 없습니다.

Danger 봇은 MR이 닿는 각 코드베이스 영역에 대한 리뷰어와 메인테이너를 선택합니다. 더 적합한 사람을 알고 있다면 제안을 재정의하세요.

룰렛은 상태에 OOO, PTO, Parental Leave, Friends and Family, Conference가 포함되거나 검토 용량에 도달한(숫자 상태 이모지 2️⃣–5️⃣로 설정) 사람을 건너뜁니다.

승인 체크리스트#

이 체크리스트는 머지 리퀘스트(MR)의 작성자, 리뷰어, 메인테이너가 품질, 성능, 신뢰성, 보안, 가관측성, 유지보수성에 대한 고영향 위험에 대해 변경 사항을 분석했는지 확인하도록 권장합니다.

체크리스트를 사용하면 소프트웨어 엔지니어링의 품질이 향상됩니다. 이 체크리스트는 GitLab 코드베이스 기여자의 역량을 지원하고 강화하는 간단한 도구입니다.

품질#

추가 품질 가이드라인은 테스팅을 참조하세요.

  • 코드 리뷰 가이드라인에 따라 이 MR을 셀프 리뷰했습니다.

  • 코드가 소프트웨어 디자인 가이드라인을 따릅니다.

  • 테스팅 피라미드에 따라 자동화된 테스트가 존재하는지 확인하세요. 누락된 테스트를 추가하거나 테스팅 격차를 문서화하는 이슈를 생성하세요.

  • GitLab.com, Dedicated 및 self-managed에 대한 기술적 영향을 고려했습니다.

  • 적절한 경우 시스템의 프론트엔드, 백엔드, 데이터베이스 부분에 대한 이 변경의 영향을 고려하고 ~ux, ~frontend, ~backend, ~database 라벨을 적용했습니다.

  • 지원되는 모든 브라우저에서 이 MR을 테스트했거나, 이 테스트가 필요하지 않다고 판단했습니다.

  • 이 변경이 업데이트 전반에 걸쳐 하위 호환성을 가지는지 확인했거나, 해당되지 않는다고 판단했습니다.

  • EE 콘텐츠를(있는 경우) FOSS에서 올바르게 분리했습니다. FOSS 컨텍스트에서 CI 파이프라인 실행을 고려하세요.

  • 기존 데이터가 예상치 못하게 다양할 수 있음을 고려했습니다. 예를 들어, 새로운 모델 유효성 검사를 추가하는 경우 기존 데이터에서는 선택 사항으로 만드는 것을 고려하세요.

  • 이 MR과 관련된 불안정한 테스트를 수정했거나, 무시해도 되는 이유를 설명했습니다. 불안정한 테스트는 오류 Flaky test '<path/to/test>' was found in the list of files changed by this MR.이 있지만 경고와 함께 통과하는 job에 있을 수 있습니다.

성능, 신뢰성 및 가용성#

이 MR에 데이터베이스 관련 변경 사항이 있나요?

  • 이 변경의 가용성 및 신뢰성 위험을 고려했습니다.

  • 미래 예상 성장에 기반한 확장성 위험을 고려했습니다.

  • 평균 고객보다 훨씬 많은 데이터를 가질 수 있는 대규모 고객에 대한 이 변경의 성능, 신뢰성, 가용성 영향을 고려했습니다.

  • 최소 시스템에서 GitLab을 실행하는 고객에 대한 이 변경의 성능, 신뢰성, 가용성 영향을 고려했습니다.

  • 이 변경이 Cells 아키텍처와 호환되는지 확인했습니다. 자세한 내용은 Cells 개발 원칙을 참조하세요.

가관측성 계측#

  • 가관측성을 통한 디버깅 및 사전 성능 개선을 용이하게 하기 위한 충분한 계측을 포함했습니다. 기능 플래그, 로깅, 계측 추가 예시를 참조하세요.

문서#

  • 변경 로그 트레일러를 포함했거나, 필요하지 않다고 판단했습니다.

이 MR에 변경 로그가 필요한가요?

  • 문서를 추가/업데이트했거나, 이 MR에 문서 변경이 불필요하다고 판단했습니다.

문서가 필요한가요?

보안#

  • 이 MR에 자격 증명 또는 토큰의 처리 또는 저장, 인가, 인증 방법 또는 보안 리뷰 가이드라인에 설명된 기타 항목에 대한 변경 사항이 포함된 경우 ~security 라벨을 추가하고 @gitlab-com/gl-security/appsec를 @ 멘션했음을 확인했습니다.

  • 이 변경에 대한 보안 검토가 필요한지 언제 그리고 어떻게 요청하는지에 대한 내부 애플리케이션 보안 리뷰에 관한 문서를 검토했으며, 필요한 경우 보안 검토를 요청했습니다.

  • MR을 차단하는 보안 스캔 결과가 있는 경우(머지 리퀘스트 승인 정책 때문에):

진양성 결과의 경우, 머지 리퀘스트가 병합되기 전에 수정되어야 합니다. 이렇게 하면 머지 리퀘스트 승인 정책에서 요구하는 AppSec 승인이 제거됩니다.

  • 위음성 결과, 위험 수용을 위해 논의해야 할 사항, 또는 의심스러운 사항의 경우 @gitlab-com/gl-security/appsec에 ping하세요.

배포#

  • 변경이 고위험일 수 있으므로 이 변경에 기능 플래그를 사용하는 것을 고려했습니다.

  • 기능 플래그를 사용하는 경우, 프로덕션에서 테스트하기 전에 스테이징에서 변경을 테스트할 계획이며, 모든 고객에게 롤아웃하기 전에 프로덕션 고객의 일부에게 먼저 롤아웃하는 것을 고려했습니다.

기능 플래그를 사용해야 하는 경우

  • 완료 정의에 따라 기본 설정 또는 새 설정 변경에 대해 인프라 부서에 알렸거나, 불필요하다고 판단했습니다.

규정 준수#

코드 리뷰 참여#

  • 친절하게 대하세요.

  • 많은 프로그래밍 결정이 의견임을 인정하세요. 트레이드오프를 논의하고 빠르게 해결하세요.

  • 질문하세요. 요구가 아닌 제안을 하세요.

  • 명시적으로 표현하세요. 사람들이 온라인에서 여러분의 의도를 항상 이해하지는 않습니다.

  • 겸손하세요. 긴 오해에 대해서는 1대1 통화를 고려하고 후속 요약을 작성하세요.

  • 댓글이 특정 사람을 향한 경우 직접 해당 인물을 멘션하세요.

  • 첫 번째 푸시 전에 전체 diff를 읽어보세요. 관련 없는 변경 사항과 디버그 코드를 확인하세요.

  • 머지 리퀘스트 가이드라인에 따라 상세한 설명을 작성하세요.

  • 피드백을 개인적으로 받아들이지 마세요. 검토는 코드와 프로덕션 시스템에 대한 영향에 관한 것입니다.

  • 코드가 하는 일만이 아니라 코드가 존재하는 이유를 설명하세요.

  • 모든 댓글에 응답하려고 노력하세요. 완전히 처리한 스레드만 해결하세요. 후속 이슈에서 댓글을 처리할 수 있다면 메인테이너와 앞으로 나아갈 방법을 협의하세요.

  • 다음 라운드 준비가 되면 다시 검토를 요청하세요.

  • 사람 리뷰어에게 검토를 요청하기 전에 GitLab Duo 검토 댓글을 모두 처리하세요.

작성자를 위한 변경 사항 빠르게 병합받기#

  • 모범 사례를 따르세요: 명확한 설명 작성, 스크린샷 및 검증 단계 추가, dangerbot 댓글 처리, 승인 체크리스트 완료.

  • GitLab 패턴을 따르세요. 긴 논의는 병합을 지연시킵니다. 문서화된 접근 방식을 따른 다음, 모범 사례 변경을 제안하는 별도의 MR을 여는 것을 고려하세요.

  • MR을 작게 유지하세요. 약 200줄이 좋은 목표입니다.

더 작은 MR은 더 빠르게 검토되고 차단 논의가 적습니다.

  • 순차적 MR에는 스택된 diff를 사용하세요.

  • 메인테이너가 MR당 하나만 필요하도록 변경을 분리하세요(예: 기능보다 먼저 데이터베이스 변경 배포).

  • 모의 데이터가 있는 UI는 기능 플래그 뒤에 있어야 합니다.

  • 리뷰어 수를 최소화하세요. 데이터베이스 리뷰어도 백엔드를 검토할 수 있으며, 풀스택 엔지니어는 프론트엔드와 백엔드를 모두 처리할 수 있습니다.

  • 메인테이너를 파악하고 도메인 전문가를 할당하세요. 메인테이너는 자신이 잘 아는 영역의 MR을 우선시합니다.

머지 리퀘스트 병합#

병합 전에:

  • 마일스톤을 설정하세요.

  • 올바른 MR 유형 라벨이 적용되었는지 확인하세요.

  • Danger 봇, 코드 품질 및 기타 보고서의 경고 및 오류를 해결하세요. 실패한 job이 있는 경우 병합 시 댓글을 작성하세요.

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

최종 승인자가 자동 병합을 설정하지 않은 경우, 필요한 모든 승인이 완료되고 병합 권한이 있으면 MR 작성자가 자신의 MR을 병합할 수 있습니다. 이는 GitLab의 행동 편향 가치에 부합합니다.

병합 준비가 되면:

머지 리퀘스트가 포크에서 온 경우 [커뮤니티 기여를 위한 가이드라인](/19.1/development/code_review/#community-contributions)도 확인하세요.

  • 작성자가 설정했거나 커밋 히스토리가 지저분한 경우에만 스쿼시 및 병합을 사용하세요.

  • 파이프라인 탭에서 파이프라인 실행을 선택한 다음 개요 탭에서 자동 병합을 활성화하세요.

기본 브랜치가 깨진 경우 특정 경우를 제외하고 병합하지 마세요.

  • 최신 파이프라인이 승인 전에 생성되었고 MR에 백엔드 변경 사항이 있는 경우 새 파이프라인을 시작하세요.

  • 최신 병합 결과 파이프라인이 16시간 미만 전에 생성된 경우(안정 브랜치의 경우 72시간) 새 파이프라인을 건너뛸 수 있습니다.

커뮤니티 기여#

**[병합 결과 파이프라인](/19.1/ci/pipelines/merge_request_pipelines/#run-pipelines-in-the-parent-project)을 시작하기 전에 악성 코드에 대해 모든 변경 사항을 철저히 검토하세요.**

커뮤니티 MR을 검토할 때:

  • 새로운 의존성(예: Gemfile.lock, yarn.lock)을 면밀히 검토하세요. 악성 패키지를 도입할 수 있습니다.

  • 특히 문서 MR에서 링크와 이미지를 검토하세요.

  • 의심스러운 경우 파이프라인을 시작하기 전에 @gitlab-com/gl-security/appsec에 검토를 요청하세요.

  • MR이 현재 마일스톤에서 병합될 가능성이 있을 때만 마일스톤을 설정하세요.

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

MR에 변경이 필요하지만 작성자가 응답하지 않거나 완료할 수 없는 경우:

  • 여러분(머지 리퀘스트 코치)이 인수한다는 댓글을 남기세요.

  • ~"coach will finish" 라벨을 추가하세요.

  • main에서 기능 브랜치를 생성하고 그들의 브랜치를 그 브랜치에 병합하세요.

  • 새로운 MR을 열고 커뮤니티 MR을 링크하고 ~"Community contribution" 라벨을 추가하세요.

  • 기여자에게 알리고 일반 검토 프로세스를 따르세요.

올바른 균형 찾기#

검토를 얼마나 깊이 할지의 올바른 균형을 찾으려면 합리적인 판단이 필요합니다. 다음을 염두에 두세요:

  • 버그를 찾는 것이 중요하지만 좋은 설계는 미래의 복잡성을 줄입니다.

  • 검토 댓글이 아닌 자동화를 통해 코드 스타일을 적용하세요.

  • 비차단 제안의 경우 다시 전달하기 전에 MR을 승인하는 것을 고려하세요. 이렇게 하면 병합 시간이 단축됩니다.

  • 일을 올바르게 하는 것과 지금 당장 올바르게 하는 것을 구분하세요. 예를 들어, 긴급한 보안 수정에서 대규모 리팩토링을 요구하는 것은 피하세요.

  • 오늘 일을 잘 하는 것이 보통 내일 완벽하게 하는 것보다 낫습니다.

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

  • 관련 없는 테스트 실패: 기본 브랜치에서도 실패가 발생하는지 확인하세요. 그렇다면 깨진 마스터 수정을 기다린 다음 MR에 대한 파이프라인을 다시 실행하세요.

  • danger-review job 실패: MR에 20개 이상의 커밋이 있는지 확인하세요. 그렇다면 리베이스하고 스쿼시하세요. 그렇지 않으면 job을 다시 실행하세요.

도움이 필요하면 MR에 @gitlab-bot help를 댓글로 작성하거나 Community Discord contribute 채널에서 질문하세요.

크레딧#

thoughtbot 코드 리뷰 가이드를 기반으로 합니다.

코드 리뷰 가이드라인

GitLab v19.1
원문 보기
요약

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

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

머지 리퀘스트 검토, 승인 및 병합 받기#

시작하기 전에 기여 승인 기준을 숙지하세요.

그룹 소속 리뷰어 또는 도메인 전문가에게 코드 검토를 받으세요.

작고 단순한 변경 사항의 경우 리뷰어 단계를 건너뛰고 메인테이너에게 직접 요청할 수 있습니다. 작고 단순한 변경의 예시:

  • 오탈자 수정 또는 소규모 텍스트 변경.

  • 동작을 변경하지 않는 소규모 리팩토링.

  • 한 달 이상 기본 활성화된 기능 플래그 제거.

  • 사용하지 않는 메서드나 클래스 제거.

  • 5줄 미만의 코드 변경이 필요한, 잘 이해된 로직 변경.

그 외의 경우, 메인테이너에게 전달하기 전에 MR이 닿는 각 카테고리의 리뷰어 승인을 받으세요. 보안 지원이 필요한 경우 @gitlab-com/gl-security/appsec를 포함하세요.

리뷰어가 승인한 후 메인테이너가 검토하고 병합합니다. 마지막으로 필요한 승인자가 병합합니다.

CODEOWNERS 필수 승인의 경우, 일반 승인보다 도메인별 승인을 먼저 구하세요. 메인테이너이기도 한 도메인별 승인자는 두 측면을 모두 검토하고 한 번만 승인해야 합니다.

승인 가이드라인#

아래 메인테이너 책임 섹션에서 설명하는 바와 같이, 도메인 전문성을 보유한 메인테이너에게 머지 리퀘스트 승인 및 병합을 받도록 권장합니다. 첫 번째 리뷰어의 선택적 승인은 여기서 다루지 않습니다. 그러나 머지 리퀘스트는 개요 섹션에서 설명한 대로 메인테이너에게 전달하기 전에 리뷰어의 검토를 받아야 합니다.

머지 리퀘스트에 포함된 내용 필요한 승인자
~backend 변경 사항 1 백엔드 메인테이너.
~database 마이그레이션 또는 비용이 많이 드는 쿼리 변경 사항 2 데이터베이스 메인테이너. 자세한 내용은 데이터베이스 리뷰 가이드라인을 참조하세요.
~workhorse 변경 사항 Workhorse 메인테이너.
~frontend 변경 사항 1 프론트엔드 메인테이너.
~UX 사용자 대면 변경 사항 3 프로덕트 디자이너. 자세한 내용은 디자인 및 사용자 인터페이스 가이드라인을 참조하세요.
새로운 JavaScript 라이브러리 추가 1 - 라이브러리가 번들 크기를 크게 증가시키는 경우 프론트엔드 디자인 시스템 멤버.- 새 라이브러리에서 사용하는 라이선스가 GitLab 사용에 승인되지 않은 경우 법무 부서 멤버. 라이선스 호환성에 대한 자세한 정보는 GitLab 라이선스 및 호환성 문서를 참조하세요.
새로운 의존성 또는 파일 시스템 변경 - Distribution 팀 멤버. 자세한 내용은 Distribution 팀과의 협력 방법을 참조하세요.- RubyGems의 경우 AppSec 리뷰를 요청하세요.
~documentation 또는 ~UI 텍스트 변경 해당 DevOps Stage 그룹 담당 기술 작성자.
개발 가이드라인 변경 검토 프로세스에 따라 해당 승인을 받으세요.
엔드투엔드 및 비엔드투엔드 변경 사항 4 테스트 소프트웨어 엔지니어.
엔드투엔드 변경 사항만 4 또는 MR 작성자가 테스트 소프트웨어 엔지니어인 경우 Quality 메인테이너.
새로운 또는 업데이트된 애플리케이션 제한 프로덕트 매니저.
Analytics Instrumentation(원격 측정 또는 분석) 변경 사항 Analytics Instrumentation 엔지니어.
GitLab에 새로운 서비스 추가(Puma, Sidekiq, Gitaly 등) 프로덕트 매니저. 자세한 내용은 GitLab에 서비스 컴포넌트 추가 프로세스를 참조하세요.
인증 관련 변경 사항 Manage:Authentication. 자세한 내용은 그룹 페이지의 코드 리뷰 섹션을 확인하세요. 팀 검토가 필요한 것으로 알려진 파일 패턴은 CODEOWNERS 파일의 Authentication 섹션에 나열되어 있으며, 해당 파일을 수정하는 모든 머지 리퀘스트의 승인자 섹션에 팀이 포함됩니다.
커스텀 역할 또는 정책 관련 변경 사항 Manage:Authorization 엔지니어.

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 로직 정확성을 위한 백엔드 검토와 컴포넌트 통합을 위한 프론트엔드 검토를 모두 받으면 좋습니다.

머지 리퀘스트가 잠재적으로 비용이 많이 드는 쿼리를 도입하는 경우 데이터베이스 메인테이너에게 지도를 구하도록 권장합니다. SQL 쿼리와 함께 해당 코드 라인에 댓글을 달면 조언을 받기 가장 효율적입니다.

사용자 대면 변경 사항에는 시각적 변경(아무리 사소하더라도)과 스크린 리더가 콘텐츠를 안내하는 방식에 영향을 미치는 렌더링된 DOM 변경이 모두 포함됩니다. 전담 프로덕트 디자이너가 없는 그룹은 커뮤니티 기여가 아닌 한 기능 변경에 대해 프로덕트 디자이너의 승인이 필요하지 않습니다.

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

머지 리퀘스트 검토#

변경이 필요한 이유(버그 수정, 사용자 경험 개선, 기존 코드 리팩토링)를 이해하세요. 그런 다음:

  • 반복 횟수를 줄이기 위해 철저하게 검토하세요.

  • 강하게 생각하는 아이디어와 그렇지 않은 아이디어를 구분해서 소통하세요.

  • 문제를 해결하면서도 코드를 단순화할 방법을 찾으세요.

  • 대안 구현을 제안하되, 작성자가 이미 고려했다고 가정하세요. ("여기서 커스텀 유효성 검사기를 사용하는 것은 어떻게 생각하세요?")

  • 작성자의 관점을 이해하려고 노력하세요.

  • 브랜치를 체크아웃하고 변경 사항을 로컬에서 테스트하세요. 상당한 GDK 수정이 필요한 MR의 경우, 스크린샷, 동영상 또는 도메인 전문가 검증을 요청하는 것을 고려하세요. 테스트를 통해 자동화된 테스트를 추가할 기회를 얻을 수 있습니다.

  • 코드를 이해하지 못하면 그렇다고 말하세요.

  • Conventional Comment 형식을 사용하여 의도를 전달하세요. 비필수 제안은 (**non-blocking:**)으로 표시하세요. 비차단 제안만 남은 경우, 기다리지 말고 MR을 다음 단계로 이동하세요.

  • 열린 의존성이 없는지 확인하세요. 차단 요소가 있는지 연결된 이슈를 확인하세요. 열린 MR에 의해 차단된 경우 MR 의존성을 설정하세요.

  • 라인 노트 한 라운드 후 "괜찮아 보입니다" 또는 "몇 가지 처리할 사항이 있습니다" 같은 요약 노트를 작성하세요.

  • 검토 후 변경이 필요한 경우 작성자에게 알려주세요.

    머지 리퀘스트가 포크에서 온 경우 커뮤니티 기여에 대한 추가 가이드라인도 확인하세요.

GitLab 특정 고려 사항#

GitLab은 Omnibus 패키지부터 소스 설치까지 많은 곳에서 사용됩니다. GitLab.com 자체는 대규모 Enterprise Edition 인스턴스입니다. 이에 따른 몇 가지 시사점이 있습니다:

  • 쿼리 변경 사항은 GitLab.com 규모에서 성능 저하가 발생하지 않는지 테스트해야 합니다. 데이터베이스 리뷰 가이드라인을 참조하세요.

  • 데이터베이스 마이그레이션은 다음을 충족해야 합니다:

되돌릴 수 있어야 합니다.

  • GitLab.com 규모에서 성능이 우수해야 합니다. 확실하지 않으면 메인테이너에게 스테이징 환경에서 마이그레이션을 테스트해 달라고 요청하세요.

  • 올바른 마이그레이션 유형이어야 합니다. 어떤 마이그레이션 유형을 선택해야 하는지에 대한 지침을 참조하세요.

  • Sidekiq 워커는 하위 호환되지 않는 방식으로 변경할 수 없습니다.

  • 캐시된 값은 릴리스 전반에 걸쳐 지속될 수 있습니다. 캐시된 값이 반환하는 유형을 변경하는 경우(예: 문자열 또는 nil에서 배열로), 동시에 캐시 키를 변경하세요.

  • 설정은 최후의 수단으로 추가해야 합니다. GitLab Rails에 새 설정 추가를 참조하세요.

  • 파일 시스템 접근은 클라우드 네이티브 아키텍처에서는 불가능합니다. 수행해야 하는 모든 파일 스토리지에 대해 오브젝트 스토리지를 지원하는지 확인하세요. 자세한 내용은 업로드 문서를 참조하세요.

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

최선의 솔루션을 찾는 직접 책임 개인(DRI)은 바로 여러분입니다. 검토 라이프사이클 전반에 걸쳐 담당자로 남아 있으세요. 스스로 담당자로 설정할 수 없는 경우, 리뷰어에게 요청하세요.

너무 크거나, 여러 이슈를 수정하거나, 복잡성이 높거나, 둘 이상의 기능을 구현하는 머지 리퀘스트를 제출하지 마세요.

MR이 여러 CODEOWNERS 섹션에 걸쳐 있는 경우, 필요한 승인 수를 최소화하기 위해 관심사별로 MR을 분리하여 검토를 병렬화하는 것을 고려하세요. 메인테이너 검토를 요청하기 전에 다음을 확인하세요:

  • MR이 가장 적절한 방식으로 의도한 문제를 해결하는지.

  • 모든 요구 사항이 충족되었는지.

  • 남은 버그, 로직 문제, 처리되지 않은 엣지 케이스 또는 알려진 취약점이 없는지.

코드 리뷰 가이드라인에 따라 MR을 셀프 리뷰하세요. 결정이나 트레이드오프를 한 라인이나 리뷰어가 코드를 이해하는 데 컨텍스트가 도움이 되는 라인에 인라인 댓글을 추가하세요.

적절한 경우 도메인 전문가, 프로덕트 매니저, UX 디자이너, 데이터베이스 전문가를 참여시키세요. MR에 도메인 전문가 검토가 필요한지 확실하지 않으면 필요한 것입니다.

10개 이상의 MR에 걸친 기능의 경우 EM 또는 Staff 엔지니어와 협력하여 컨텍스트를 공유하는 일관된 메인테이너를 식별하세요.

MR이 여러 도메인에 걸쳐 있는 경우, 각 도메인 전문가에게 검토를 요청하세요.

검토를 요청하기 전에 다음에 대한 MR diff 댓글을 추가하세요:

  • 추가된 린팅 규칙(RuboCop, JS 등).

  • 추가된 라이브러리(Ruby 젬, JS 라이브러리 등).

  • 명확하지 않은 경우 부모 클래스나 메서드에 대한 링크.

  • 벤치마킹 결과.

  • 잠재적으로 보안에 취약한 코드.

리뷰어가 솔루션을 검증하는 데 필요한 프로젝트, 스니펫 또는 자산에 접근할 수 있는지 확인하세요.

리뷰어를 할당할 때 각 리뷰어가 집중해야 할 도메인을 명시하는 댓글을 남기세요. 이는 팀 멤버가 여러 영역에 전문 지식을 가지고 있을 때 모호함을 방지합니다. 예시는 MR 75921과 MR 109500을 참조하세요.

리뷰어가 요구하는 경우에만 소스 코드에 TODO 댓글을 추가하세요. TODO를 추가하는 경우 관련 이슈에 대한 링크를 포함하세요.

왜, 무엇을만이 아닌 코드가 하는 일을 설명하는 댓글을 작성하세요.

테스트가 통과할 때만 메인테이너 검토를 요청하세요. 테스트가 실패하는 경우 댓글에 이유를 설명하세요. 즉각적인 요청의 경우에만 이메일이나 Slack으로 메인테이너에게 연락하세요. 그 외의 경우 리뷰어로 추가하는 것으로 충분합니다.

리뷰어의 책임#

리뷰어는 선택된 솔루션의 세부 사항을 검토할 책임이 있습니다.

검토 응답 SLO 내에 응답이 불가능한 경우, 작성자에게 알리고 검토 워크로드 대시보드를 사용하여 대체자를 찾아 할당하세요.

MR이 모든 기여 승인 기준을 충족한다고 확신하는 경우:

  • 승인을 선택하세요.

  • 작성자에게 알리기 위해 @ 멘션하세요.

  • 도메인 전문성을 가진 메인테이너에게 검토를 요청하거나 리뷰어 룰렛 제안을 따르세요.

메인테이너의 책임#

메인테이너는 GitLab 코드베이스의 전반적인 건강, 품질 및 일관성에 대한 책임이 있습니다. 검토는 아키텍처, 코드 구성, 관심사 분리, 테스트, DRYness, 일관성 및 가독성에 초점을 맞춥니다.

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

메인테이너는 MR의 영향을 평가할 때 합리적인 판단을 내립니다. 메인테이너가 MR을 병합할 수 없다고 판단하면 그렇게 말하는 것이 그들의 책임입니다. 메인테이너는 또한 두 번째 의견을 위해 다른 사람을 참여시켜야 할 때를 아는 전문 자문가입니다.

메인테이너가 MR을 승인하면 작성자와 함께 책임을 지게 됩니다. 이는 프로덕션 사고가 발생했을 때 메인테이너가 문제 해결을 위해 호출될 수 있음을 의미합니다.

특정 머지 리퀘스트는 안정 브랜치를 대상으로 할 수 있습니다. 이러한 요청을 처리하는 방법에 대한 개요는 패치 릴리즈 런북을 참조하세요.

모범 사례#

도메인 전문가#

도메인 전문가는 특정 기술, 제품 기능 또는 코드베이스 영역에 상당한 경험을 가진 팀 멤버입니다. 팀 멤버는 자신을 도메인 전문가로 자가 식별하고 팀 프로필에 추가하도록 권장됩니다.

도메인 전문가로 자가 식별할 때는 .yml 파일을 변경하는 MR을 이미 확립된 도메인 전문가 또는 해당 엔지니어링 매니저가 병합하도록 할당하는 것이 권장됩니다.

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

  • 특정 Stage/그룹(예: create: source code)에서 일하는 팀 멤버는 그들이 작업하는 앱 영역의 도메인 전문가로 간주됩니다.

  • 특정 기능(예: 검색)에서 일하는 팀 멤버는 해당 기능의 도메인 전문가로 간주됩니다.

코드 리뷰의 경우 기본적으로 도메인 전문성을 가진 팀 멤버에게 검토를 할당합니다. UX 검토는 기본적으로 검토 룰렛에서 추천된 리뷰어로 합니다. 디자이너 용량 제한으로 인해 프로덕트 디자이너가 지원하지 않는 영역은 커뮤니티 기여가 아닌 한 더 이상 UX 검토가 필요하지 않습니다. 적합한 도메인 전문가를 구할 수 없는 경우 어떤 팀 멤버든 MR을 검토하도록 선택하거나 리뷰어 룰렛 추천을 따를 수 있습니다(UX 검토에 대해서는 위를 참조). 할당하기 전에 해당 인물이 부재중인지 확인하세요.

도메인 전문가를 찾으려면:

  • 머지 리퀘스트 승인 위젯에서 적격 승인자 보기를 선택하세요. 이 위젯은 코드베이스의 각 영역별 권장 및 필수 승인을 보여줍니다. 이러한 규칙은 Code Owners에 정의되어 있습니다.

  • 머지 리퀘스트와 관련된 Stage 또는 그룹에서 일하는 팀 멤버 목록을 확인하세요.

  • 엔지니어링 프로젝트 페이지 또는 GitLab 팀 페이지에서 팀 멤버의 도메인 전문성을 확인하세요. 도메인은 자가 식별되므로 머지 리퀘스트의 변경 사항을 도메인에 매핑할 때 판단을 활용하세요.

  • 머지 리퀘스트의 파일에 기여한 팀 멤버를 찾으세요. git log <file>을 실행하여 로그를 확인하세요.

  • 파일을 검토한 팀 멤버를 찾으세요. 관련 머지 리퀘스트를 다음 방법으로 찾을 수 있습니다:

git log <file>을 사용하여 커밋 SHA를 가져오세요.

  • https://gitlab.com/gitlab-org/gitlab/-/commit/로 이동하세요.

  • 커밋에 대해 표시된 관련 머지 리퀘스트를 선택하세요.

리뷰어 룰렛#

[리뷰어 룰렛](https://gitlab-org.gitlab.io/gitlab-roulette/)은 GitLab.com용 내부 도구로, 고객 설치에서는 사용할 수 없습니다.

Danger 봇은 MR이 닿는 각 코드베이스 영역에 대한 리뷰어와 메인테이너를 선택합니다. 더 적합한 사람을 알고 있다면 제안을 재정의하세요.

룰렛은 상태에 OOO, PTO, Parental Leave, Friends and Family, Conference가 포함되거나 검토 용량에 도달한(숫자 상태 이모지 2️⃣–5️⃣로 설정) 사람을 건너뜁니다.

승인 체크리스트#

이 체크리스트는 머지 리퀘스트(MR)의 작성자, 리뷰어, 메인테이너가 품질, 성능, 신뢰성, 보안, 가관측성, 유지보수성에 대한 고영향 위험에 대해 변경 사항을 분석했는지 확인하도록 권장합니다.

체크리스트를 사용하면 소프트웨어 엔지니어링의 품질이 향상됩니다. 이 체크리스트는 GitLab 코드베이스 기여자의 역량을 지원하고 강화하는 간단한 도구입니다.

품질#

추가 품질 가이드라인은 테스팅을 참조하세요.

  • 코드 리뷰 가이드라인에 따라 이 MR을 셀프 리뷰했습니다.

  • 코드가 소프트웨어 디자인 가이드라인을 따릅니다.

  • 테스팅 피라미드에 따라 자동화된 테스트가 존재하는지 확인하세요. 누락된 테스트를 추가하거나 테스팅 격차를 문서화하는 이슈를 생성하세요.

  • GitLab.com, Dedicated 및 self-managed에 대한 기술적 영향을 고려했습니다.

  • 적절한 경우 시스템의 프론트엔드, 백엔드, 데이터베이스 부분에 대한 이 변경의 영향을 고려하고 ~ux, ~frontend, ~backend, ~database 라벨을 적용했습니다.

  • 지원되는 모든 브라우저에서 이 MR을 테스트했거나, 이 테스트가 필요하지 않다고 판단했습니다.

  • 이 변경이 업데이트 전반에 걸쳐 하위 호환성을 가지는지 확인했거나, 해당되지 않는다고 판단했습니다.

  • EE 콘텐츠를(있는 경우) FOSS에서 올바르게 분리했습니다. FOSS 컨텍스트에서 CI 파이프라인 실행을 고려하세요.

  • 기존 데이터가 예상치 못하게 다양할 수 있음을 고려했습니다. 예를 들어, 새로운 모델 유효성 검사를 추가하는 경우 기존 데이터에서는 선택 사항으로 만드는 것을 고려하세요.

  • 이 MR과 관련된 불안정한 테스트를 수정했거나, 무시해도 되는 이유를 설명했습니다. 불안정한 테스트는 오류 Flaky test '<path/to/test>' was found in the list of files changed by this MR.이 있지만 경고와 함께 통과하는 job에 있을 수 있습니다.

성능, 신뢰성 및 가용성#

이 MR에 데이터베이스 관련 변경 사항이 있나요?

  • 이 변경의 가용성 및 신뢰성 위험을 고려했습니다.

  • 미래 예상 성장에 기반한 확장성 위험을 고려했습니다.

  • 평균 고객보다 훨씬 많은 데이터를 가질 수 있는 대규모 고객에 대한 이 변경의 성능, 신뢰성, 가용성 영향을 고려했습니다.

  • 최소 시스템에서 GitLab을 실행하는 고객에 대한 이 변경의 성능, 신뢰성, 가용성 영향을 고려했습니다.

  • 이 변경이 Cells 아키텍처와 호환되는지 확인했습니다. 자세한 내용은 Cells 개발 원칙을 참조하세요.

가관측성 계측#

  • 가관측성을 통한 디버깅 및 사전 성능 개선을 용이하게 하기 위한 충분한 계측을 포함했습니다. 기능 플래그, 로깅, 계측 추가 예시를 참조하세요.

문서#

  • 변경 로그 트레일러를 포함했거나, 필요하지 않다고 판단했습니다.

이 MR에 변경 로그가 필요한가요?

  • 문서를 추가/업데이트했거나, 이 MR에 문서 변경이 불필요하다고 판단했습니다.

문서가 필요한가요?

보안#

  • 이 MR에 자격 증명 또는 토큰의 처리 또는 저장, 인가, 인증 방법 또는 보안 리뷰 가이드라인에 설명된 기타 항목에 대한 변경 사항이 포함된 경우 ~security 라벨을 추가하고 @gitlab-com/gl-security/appsec를 @ 멘션했음을 확인했습니다.

  • 이 변경에 대한 보안 검토가 필요한지 언제 그리고 어떻게 요청하는지에 대한 내부 애플리케이션 보안 리뷰에 관한 문서를 검토했으며, 필요한 경우 보안 검토를 요청했습니다.

  • MR을 차단하는 보안 스캔 결과가 있는 경우(머지 리퀘스트 승인 정책 때문에):

진양성 결과의 경우, 머지 리퀘스트가 병합되기 전에 수정되어야 합니다. 이렇게 하면 머지 리퀘스트 승인 정책에서 요구하는 AppSec 승인이 제거됩니다.

  • 위음성 결과, 위험 수용을 위해 논의해야 할 사항, 또는 의심스러운 사항의 경우 @gitlab-com/gl-security/appsec에 ping하세요.

배포#

  • 변경이 고위험일 수 있으므로 이 변경에 기능 플래그를 사용하는 것을 고려했습니다.

  • 기능 플래그를 사용하는 경우, 프로덕션에서 테스트하기 전에 스테이징에서 변경을 테스트할 계획이며, 모든 고객에게 롤아웃하기 전에 프로덕션 고객의 일부에게 먼저 롤아웃하는 것을 고려했습니다.

기능 플래그를 사용해야 하는 경우

  • 완료 정의에 따라 기본 설정 또는 새 설정 변경에 대해 인프라 부서에 알렸거나, 불필요하다고 판단했습니다.

규정 준수#

코드 리뷰 참여#

  • 친절하게 대하세요.

  • 많은 프로그래밍 결정이 의견임을 인정하세요. 트레이드오프를 논의하고 빠르게 해결하세요.

  • 질문하세요. 요구가 아닌 제안을 하세요.

  • 명시적으로 표현하세요. 사람들이 온라인에서 여러분의 의도를 항상 이해하지는 않습니다.

  • 겸손하세요. 긴 오해에 대해서는 1대1 통화를 고려하고 후속 요약을 작성하세요.

  • 댓글이 특정 사람을 향한 경우 직접 해당 인물을 멘션하세요.

  • 첫 번째 푸시 전에 전체 diff를 읽어보세요. 관련 없는 변경 사항과 디버그 코드를 확인하세요.

  • 머지 리퀘스트 가이드라인에 따라 상세한 설명을 작성하세요.

  • 피드백을 개인적으로 받아들이지 마세요. 검토는 코드와 프로덕션 시스템에 대한 영향에 관한 것입니다.

  • 코드가 하는 일만이 아니라 코드가 존재하는 이유를 설명하세요.

  • 모든 댓글에 응답하려고 노력하세요. 완전히 처리한 스레드만 해결하세요. 후속 이슈에서 댓글을 처리할 수 있다면 메인테이너와 앞으로 나아갈 방법을 협의하세요.

  • 다음 라운드 준비가 되면 다시 검토를 요청하세요.

  • 사람 리뷰어에게 검토를 요청하기 전에 GitLab Duo 검토 댓글을 모두 처리하세요.

작성자를 위한 변경 사항 빠르게 병합받기#

  • 모범 사례를 따르세요: 명확한 설명 작성, 스크린샷 및 검증 단계 추가, dangerbot 댓글 처리, 승인 체크리스트 완료.

  • GitLab 패턴을 따르세요. 긴 논의는 병합을 지연시킵니다. 문서화된 접근 방식을 따른 다음, 모범 사례 변경을 제안하는 별도의 MR을 여는 것을 고려하세요.

  • MR을 작게 유지하세요. 약 200줄이 좋은 목표입니다.

더 작은 MR은 더 빠르게 검토되고 차단 논의가 적습니다.

  • 순차적 MR에는 스택된 diff를 사용하세요.

  • 메인테이너가 MR당 하나만 필요하도록 변경을 분리하세요(예: 기능보다 먼저 데이터베이스 변경 배포).

  • 모의 데이터가 있는 UI는 기능 플래그 뒤에 있어야 합니다.

  • 리뷰어 수를 최소화하세요. 데이터베이스 리뷰어도 백엔드를 검토할 수 있으며, 풀스택 엔지니어는 프론트엔드와 백엔드를 모두 처리할 수 있습니다.

  • 메인테이너를 파악하고 도메인 전문가를 할당하세요. 메인테이너는 자신이 잘 아는 영역의 MR을 우선시합니다.

머지 리퀘스트 병합#

병합 전에:

  • 마일스톤을 설정하세요.

  • 올바른 MR 유형 라벨이 적용되었는지 확인하세요.

  • Danger 봇, 코드 품질 및 기타 보고서의 경고 및 오류를 해결하세요. 실패한 job이 있는 경우 병합 시 댓글을 작성하세요.

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

최종 승인자가 자동 병합을 설정하지 않은 경우, 필요한 모든 승인이 완료되고 병합 권한이 있으면 MR 작성자가 자신의 MR을 병합할 수 있습니다. 이는 GitLab의 행동 편향 가치에 부합합니다.

병합 준비가 되면:

머지 리퀘스트가 포크에서 온 경우 [커뮤니티 기여를 위한 가이드라인](/19.1/development/code_review/#community-contributions)도 확인하세요.

  • 작성자가 설정했거나 커밋 히스토리가 지저분한 경우에만 스쿼시 및 병합을 사용하세요.

  • 파이프라인 탭에서 파이프라인 실행을 선택한 다음 개요 탭에서 자동 병합을 활성화하세요.

기본 브랜치가 깨진 경우 특정 경우를 제외하고 병합하지 마세요.

  • 최신 파이프라인이 승인 전에 생성되었고 MR에 백엔드 변경 사항이 있는 경우 새 파이프라인을 시작하세요.

  • 최신 병합 결과 파이프라인이 16시간 미만 전에 생성된 경우(안정 브랜치의 경우 72시간) 새 파이프라인을 건너뛸 수 있습니다.

커뮤니티 기여#

**[병합 결과 파이프라인](/19.1/ci/pipelines/merge_request_pipelines/#run-pipelines-in-the-parent-project)을 시작하기 전에 악성 코드에 대해 모든 변경 사항을 철저히 검토하세요.**

커뮤니티 MR을 검토할 때:

  • 새로운 의존성(예: Gemfile.lock, yarn.lock)을 면밀히 검토하세요. 악성 패키지를 도입할 수 있습니다.

  • 특히 문서 MR에서 링크와 이미지를 검토하세요.

  • 의심스러운 경우 파이프라인을 시작하기 전에 @gitlab-com/gl-security/appsec에 검토를 요청하세요.

  • MR이 현재 마일스톤에서 병합될 가능성이 있을 때만 마일스톤을 설정하세요.

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

MR에 변경이 필요하지만 작성자가 응답하지 않거나 완료할 수 없는 경우:

  • 여러분(머지 리퀘스트 코치)이 인수한다는 댓글을 남기세요.

  • ~"coach will finish" 라벨을 추가하세요.

  • main에서 기능 브랜치를 생성하고 그들의 브랜치를 그 브랜치에 병합하세요.

  • 새로운 MR을 열고 커뮤니티 MR을 링크하고 ~"Community contribution" 라벨을 추가하세요.

  • 기여자에게 알리고 일반 검토 프로세스를 따르세요.

올바른 균형 찾기#

검토를 얼마나 깊이 할지의 올바른 균형을 찾으려면 합리적인 판단이 필요합니다. 다음을 염두에 두세요:

  • 버그를 찾는 것이 중요하지만 좋은 설계는 미래의 복잡성을 줄입니다.

  • 검토 댓글이 아닌 자동화를 통해 코드 스타일을 적용하세요.

  • 비차단 제안의 경우 다시 전달하기 전에 MR을 승인하는 것을 고려하세요. 이렇게 하면 병합 시간이 단축됩니다.

  • 일을 올바르게 하는 것과 지금 당장 올바르게 하는 것을 구분하세요. 예를 들어, 긴급한 보안 수정에서 대규모 리팩토링을 요구하는 것은 피하세요.

  • 오늘 일을 잘 하는 것이 보통 내일 완벽하게 하는 것보다 낫습니다.

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

  • 관련 없는 테스트 실패: 기본 브랜치에서도 실패가 발생하는지 확인하세요. 그렇다면 깨진 마스터 수정을 기다린 다음 MR에 대한 파이프라인을 다시 실행하세요.

  • danger-review job 실패: MR에 20개 이상의 커밋이 있는지 확인하세요. 그렇다면 리베이스하고 스쿼시하세요. 그렇지 않으면 job을 다시 실행하세요.

도움이 필요하면 MR에 @gitlab-bot help를 댓글로 작성하거나 Community Discord contribute 채널에서 질문하세요.

크레딧#

thoughtbot 코드 리뷰 가이드를 기반으로 합니다.