코드 리뷰 가이드라인
이 가이드는 코드 리뷰를 수행하고 자신의 코드를 리뷰받기 위한 조언과 모범 사례를 담고 있습니다.
GitLab CE 및 EE에 대한 모든 병합 요청은 GitLab 팀원 또는 더 넓은 커뮤니티 구성자가 작성하였든 간에, 코드가 효과적이고 이해 가능하며 유지 보수 가능하고 안전한지를 보장하기 위해 코드 리뷰 프로세스를 거쳐야 합니다.
병합 요청을 리뷰받고, 승인받고, 병합하기
시작하기 전에:
- 기여 수용 기준을 숙지하세요.
- 가이드가 필요하다면 (예를 들어, 첫 번째 병합 요청인 경우), Merge request coaches 중 한 명에게 자유롭게 질문하세요.
리뷰할 코드가 준비되면 리뷰어에게 코드를 리뷰 받으세요. 이 리뷰어는 귀하의 그룹 또는 팀의 일원이거나 도메인 전문가일 수 있습니다. 리뷰어는 다음을 할 수 있습니다:
- 선택한 해결책과 구현에 대한 두 번째 의견을 제공할 수 있습니다.
- 버그, 논리 문제 또는 드러나지 않은 엣지 케이스를 찾는 데 도움을 줄 수 있습니다.
병합 요청이 작고 리뷰하기 간단하다면, 리뷰어 단계는 건너뛰고 직접 유지 관리인에게 요청할 수 있습니다.
“작고 간단한 것”이 무엇인지에 대해서는 애매한 부분이 있습니다. 다음은 작고 간단한 변경의 몇 가지 예시입니다:
- 오타 수정 또는 작은 복사 변경 (예시).
- 동작이나 데이터를 변경하지 않는 작은 리팩토링.
-
1개월 동안 기본으로 활성화된 기능 플래그에 대한 참조 제거.
- 사용되지 않는 메서드나 클래스 제거.
- < 5줄의 코드 변경이 필요한 잘 이해된 논리 변경.
그 외에는 병합 요청이 각각의 범주 (예: 백엔드, 데이터베이스)에서 리뷰어에 의해 먼저 리뷰되어야 하며, 관리자가 관련 도메인 지식이 없을 수도 있기 때문에 이는 업무 부담을 분산시키는 데도 도움이 됩니다.
보안 스캔이나 댓글에 대한 지원이 필요하면 응용 프로그램 보안 팀(@gitlab-com/gl-security/appsec
)을 포함하세요.
리뷰어는 사이드바에서 리뷰어 기능을 사용합니다. 리뷰어는 추가 승인하여 자신의 승인을 추가할 수 있습니다.
병합 요청이 다루는 영역에 따라, 하나 이상의 유지 관리인의 승인을 받아야 합니다. 승인됨 버튼은 병합 요청 위젯에 있습니다.
병합 요청이 병합되기 위해서도 유지 관리인이 필요합니다. 여러 승인 권한이 필요한 경우, 마지막으로 리뷰하고 승인한 유지 관리인이 병합합니다.
일부 도메인 영역(Verify
와 같은)은 CODEOWNERS 규칙에 따라 도메인 전문가의 승인이 필요합니다. CODEOWNERS 섹션은 독립적인 승인 규칙이므로, 특정 규칙(Verify
등)은 다른 일반 승인 규칙(예: 백엔드
)의 하위 집합일 수 있습니다. 보다 효율적인 프로세스를 위해 저자는 일반 승인을 받기 전에 도메인별 승인을 찾아야 합니다. 도메인별 승인자는 유지 관리인일 수 있으며, 이 경우 이들은 도메인 세부 사항과 더 넓은 변경 사항을 동시에 검토하고 두 역할에 대해 한 번에 승인해야 합니다.
아래에서 저자의 책임에 대해 더 읽어보세요.
도메인 전문가
도메인 전문가는 특정 기술, 제품 특징 또는 코드베이스 영역에 대한 상당한 경험을 가진 팀원입니다.
팀원들은 도메인 전문가로 자가 식별할 것을 권장하며, 팀 프로필에 추가할 수 있습니다.
자가 식별 시, .yml
파일을 변경하는 MR은 이미 확립된 도메인 전문가 또는 해당 엔지니어링 매니저에 의해 병합되는 것이 좋습니다.
다음과 같은 가정을 합니다:
- 특정 단계/그룹에서 작업하는 팀원(예: create: source code)은 그들이 작업하는 앱 영역에 대한 도메인 전문가로 간주됩니다.
- 특정 기능(예: search)에서 작업하는 팀원은 해당 기능에 대한 도메인 전문가로 간주됩니다.
우리는 코드 리뷰를 위한 도메인 전문성을 가진 팀원에게 리뷰를 할당하는 것을 기본으로 합니다. UX 리뷰는 Review Roulette에서 추천된 리뷰어에게 기본적으로 할당됩니다. 디자이너 용량의 제한으로 인해, 제품 디자이너의 지원이 없는 영역은 커뮤니티 기여가 아닌 이상 더 이상 UX 리뷰를 요구하지 않습니다. 적절한 도메인 전문가가 없는 경우, 모든 팀원 중에서 MR을 검토할 팀원을 선택하거나 리뷰어 룰렛 추천을 따르세요(위의 UX 리뷰 참고). 사람에게 OOO 상태인지 다시 확인한 후 할당하세요.
도메인 전문가를 찾는 방법:
- 병합 요청 승인 위젯에서 자격을 갖춘 승인자 보기를 선택하세요. 이 위젯은 코드베이스의 각 영역에 대한 추천 및 필수 승인을 보여줍니다. 이러한 규칙은 코드 소유자에서 정의됩니다.
- 병합 요청과 관련된 단계 또는 그룹에서 작업하는 팀원 목록을 확인하세요.
- 엔지니어링 프로젝트 페이지 또는 GitLab 팀 페이지에서 팀원의 도메인 전문성을 확인하세요. 도메인은 자가 식별되므로, 병합 요청의 변경 사항을 도메인에 매핑할 때 당신의 판단을 사용하세요.
- 병합 요청의 파일에 기여한 팀원을 찾으세요.
git log <file>
을 실행하여 로그를 확인하세요. - 파일을 검토한 팀원을 찾으세요. 관련 병합 요청을 찾으려면:
-
git log <file>
을 사용하여 커밋 SHA를 가져옵니다. -
https://gitlab.com/gitlab-org/gitlab/-/commit/<SHA>
로 이동합니다. - 커밋에 대해 표시된 관련 병합 요청을 선택합니다.
-
리뷰어 룰렛
참고: 리뷰어 룰렛은 GitLab.com에서 사용할 수 있는 내부 도구이며, 고객 설치에서는 사용할 수 없습니다.
Danger bot는 귀하의 병합 요청이 영향을 주는 코드베이스의 각 영역에 대해 무작위로 리뷰어와 유지 관리자를 선택합니다.
개발자 리뷰어에 대한 추천을 하며, 다른 사람이 더 적합하다고 생각되면 이를 무시할 수 있습니다.
승인 가이드라인은 도메인 전문가를 선택하는 데 도움이 될 수 있습니다.
우리는 제품 디자이너를 포함한 팀에서 오는 MR에 대해서만 UX 리뷰를 수행합니다. 이러한 팀의 사용자-facing 변경 사항은 기능 플래그 뒤에 있더라도 UX 리뷰를 받아야 합니다. 기본적으로 추천된 UX 리뷰어를 따르세요.
리뷰어와 유지 관리자를 엔지니어링 프로젝트 페이지의 목록에서 선택하며, 다음과 같은 행동을 보입니다:
- 슬랙 또는 GitLab 상태에
OOO
,PTO
,Parental Leave
,Friends and Family
, 또는Conference
문자열이 포함되어 있는 사람을 선택하지 않습니다.- 이모지 항목이 다음 범주 중 하나인 경우:
-
휴가 중 - 🌴
:palm_tree:
, 🏖️:beach:
, ⛱:beach_umbrella:
, 🏖:beach_with_umbrella:
, 🌞:sun_with_face:
, 🎡:ferris_wheel:
, 🏙:cityscape:
-
병가 중 - 🌡️
:thermometer:
, 🤒:face_with_thermometer:
-
휴가 중 - 🌴
- 이모지 항목이 다음 범주 중 하나인 경우:
- 이미 선택된 리뷰 수가 자신의 선택한 “리뷰 제한”과 같거나 더 많은 사람을 선택하지 않습니다. 리뷰 제한은 사람들이 동시에 처리할 준비가 된 최대 리뷰 수입니다. 슬랙 또는 GitLab 상태를 사용하여 이 중 하나로 리뷰 제한을 설정하세요:
- 2️⃣ -
:two:
- 3️⃣ -
:three:
- 4️⃣ -
:four:
- 5️⃣ -
:five:
최소 리뷰 제한은 2️⃣입니다. 리뷰 요청을 위해 자신을 완전히 비활성화할 수 없는 이유는 이 문제에서 논의되었습니다.
어떤 프로젝트의 기본 브랜치를 목표로 하지 않는 병합 요청에 대한 리뷰 요청은 집계되지 않습니다. 이러한 MR은 일반적으로 백포트이며, 유지 관리자 또는 리뷰어는 이를 검토하는 데 큰 시간을 필요로 하지 않습니다.
- 2️⃣ -
- 항상 동일한 브랜치 이름에 대해 동일한 리뷰어와 유지 관리자를 선택합니다(단, OOO 상태가 변경되지 않는 한).
ce-
및ee-
를 선두에서 제거하고-ce
및-ee
를 뒤쪽에서 제거하여 백포트 브랜치에 대해 안정적으로 유지할 수 있습니다. - 슬랙 또는 GitLab 상태 이모지가 Ⓜ
:m:
인 사람은 자신이 유지 관리자 역할을 하는 프로젝트의 리뷰어로만 제안됩니다.
룰렛 대시보드에는:
- 지난 7일 및 30일 간의 할당 이벤트.
- 개인별 현재 할당된 병합 요청.
- 다양한 기준으로 정렬.
- 수동 리뷰어 룰렛.
- 로컬 시간 정보.
더 많은 정보를 원하신다면 룰렛 README를 검토하세요.
승인 지침
아래 유지 관리자의 책임에 대한 섹션에 설명된 대로,
여러분은 전문 분야에 대한 경험이 있는 유지 관리자가 병합 요청을 승인하고 병합하도록 권장됩니다.
첫 번째 검토자의 선택적 승인은 여기서 다루지 않습니다.
그러나 여러분의 병합 요청은 유지 관리자로 전달하기 전에 검토자가 검토해야 하며, 이는 개요 섹션에 설명되어 있습니다.
병합 요청이 포함하는 내용 | 승인해야 하는 사람 |
---|---|
~backend 변경 사항 1
|
백엔드 유지 관리인. |
~database 마이그레이션 또는 비싼 쿼리 변경 2
|
데이터베이스 유지 관리인. 더 많은 세부정보는 데이터베이스 검토 지침을 참조하세요. |
~workhorse 변경 사항 |
워크호스 유지 관리인. |
~frontend 변경 사항 1
|
프론트엔드 유지 관리인. |
~UX 사용자-facing 변경 사항 3
|
제품 디자이너. 세부정보는 디자인 및 사용자 인터페이스 지침을 참조하세요. |
새로운 JavaScript 라이브러리 추가 1 | - 프론트엔드 디자인 시스템 회원 (라이브러리가 번들 크기를 크게 증가시키는 경우). - 법무팀 회원 (새 라이브러리에서 사용하는 라이센스가 GitLab 사용 승인을 받지 않은 경우). 라이센스 호환성에 대한 추가 정보는 GitLab 라이센싱 및 호환성 문서에서 확인할 수 있습니다. |
새로운 의존성 또는 파일 시스템 변경 | - 배포 팀 회원. 배포 팀과 작업하는 방법에서 더 많은 세부정보를 확인하세요. - RubyGems의 경우, AppSec 검토 요청을 요청하세요. |
~documentation 또는 ~UI text 변경 사항 |
기술 문서 작성자 (적절한 DevOps 단계 그룹에서의 할당 기반). |
개발 지침 변경 사항 | 검토 프로세스를 따르고 이에 따라 승인을 받으세요. |
엔드 투 엔드 및 비엔드 투 엔드 변경 사항 4 | 테스트 소프트웨어 엔지니어. |
오직 엔드 투 엔드 변경 사항 4 또는 MR 작성자가 소프트웨어 엔지니어인 경우 | 품질 유지 관리인. |
새로운 또는 업데이트된 애플리케이션 제한 | 제품 관리자. |
분석 도구(텔레메트리 또는 분석) 변경 사항 | 분석 도구 엔지니어. |
Feature spec의 추가 또는 변경 사항 | 품질 유지 관리인 또는 품질 검토자. |
GitLab에 새로운 서비스 추가 (예: Puma, Sidekiq, Gitaly) | 제품 관리자. GitLab에 서비스 구성 요소를 추가하는 프로세스를 참조하세요. |
인증 관련 변경 사항 |
Manage:Authentication. 그룹 페이지의 코드 검토 섹션에서 더 많은 세부정보를 확인하세요. 팀의 검토가 필요한 파일 패턴은 CODEOWNERS 파일의 Authentication 섹션에 나열되어 있으며, 팀은 이러한 파일을 수정하는 모든 병합 요청의 승인자 섹션에 나열됩니다. |
사용자 정의 역할 또는 정책 관련 변경 사항 | Manage:Authorization Engineer. |
-
JavaScript 사양 이외의 사양은
~backend
코드로 간주됩니다. Haml 마크업은~frontend
코드로 간주됩니다. 그러나 Haml 템플릿의 Ruby 코드는~backend
코드로 간주됩니다. 의문이 있을 경우, 프론트엔드와 백엔드 모두에 대한 검토를 요청하세요. -
병합 요청이 비싼 쿼리를 도입할 가능성이 있는 경우 데이터베이스 유지 관리자의 안내를 받는 것이 좋습니다. 해당 SQL 쿼리와 함께 문제가 있는 코드 줄에 댓글을 달면 그들이 조언을 줄 수 있습니다.
-
사용자-facing 변경 사항에는 매우 사소한 사항에 관계없이 시각적 변경 사항과 화면 리더가 콘텐츠를 어떻게 발표할지에 영향을 미치는 렌더링된 DOM에 대한 변경 사항이 포함됩니다. 전담 제품 디자이너가 없는 그룹은 커뮤니티 기여를 제외하고는 기능 변경 승인에 제품 디자이너가 필요하지 않습니다.
-
엔드 투 엔드 변경 사항에는
qa
디렉토리 내의 모든 파일이 포함됩니다.
CODEOWNERS 승인
일부 병합 요청은 특정 그룹의 필수 승인이 필요합니다.
정의는 .gitlab/CODEOWNERS
를 참조하세요.
.gitlab/CODEOWNERS
에서의 필수 섹션은
다음과 같은 이유로 필요할 경우에만 제한해야 합니다:
- 준수
- 가용성
- 보안
필수 섹션을 추가할 때는 새로운 필수 섹션이 병합 요청 속도에 미치는 영향을 추적해야 합니다.
좋은 예를 보려면 문제 확인을 참조하세요.
다른 모든 경우에는 필수 섹션을 사용하지 않아야 하며, 우리는 책임이 경직성보다 중요하다고 생각합니다.
또한, 현재 모놀리식 구조는 병합 요청이 겉보기에 관련이 없는 부분에 영향을 미칠 가능성이 높습니다.
여러 개의 필수 승인은 이러한 병합 요청이 작성자로 하여금 승인을 요청해야 하므로 비효율적입니다.
이 문제를 개선하기 위한 노력은 다음과 같습니다:
- https://gitlab.com/groups/gitlab-org/-/epics/11624
- https://gitlab.com/gitlab-org/gitlab/-/issues/377326
수용 체크리스트
이 체크리스트는 병합 요청(MR)의 작성자, 리뷰어 및 유지 관리자가 품질, 성능, 신뢰성, 보안, 관측 가능성 및 유지 관리성에 대한 높은 영향 위험이 분석되었음을 확인하도록 권장합니다.
체크리스트를 사용하면 소프트웨어 엔지니어링의 품질이 향상됩니다. 이 체크리스트는 GitLab 코드베이스에 기여하는 사람들의 기술을 지원하고 강화하는 간단한 도구입니다.
품질
추가 품질 지침은 테스트 엔지니어링 프로세스를 참조하세요.
- 코드 리뷰 지침에 따라 이 MR을 자체 검토했습니다.
- 이 변경이 영향을 미치는 코드에 대해 자동화된 테스트(테스트 가이드)가 사용자에게 매우 중요한 기능을 검증한다고 믿습니다(모든 테스트 레벨을 고려합니다 테스트 레벨).
- 기존 자동화된 테스트가 위 기능을 커버하지 않는 경우, 필요한 추가 테스트를 추가했거나 자동화 테스트 간극을 설명하는 이슈를 추가하고 이를 이 MR에 링크했습니다.
- 이 변경이 GitLab.com 호스팅 고객과 자체 관리 고객에 미치는 기술적 측면을 고려했습니다.
- 이 변경이 시스템의 프런트엔드, 백엔드 및 데이터베이스 부분에 미치는 영향을 고려하고 적절히
~ux
,~frontend
,~backend
,~database
레이블을 적용했습니다. - 모든 지원되는 브라우저에서 이 MR을 테스트했거나, 이 테스트가 필요하지 않다고 판단했습니다.
- 이 변경이 업데이트 간에 호환되는지 확인했습니다, 또는 이것이 적용되지 않기로 결정했습니다.
- EE 콘텐츠를 FOSS와 제대로 분리했거나, 이 MR은 오로지 FOSS입니다.
- 이 MR이 EE와 FOSS에 다르게 영향을 미칠 수 있는 경우, FOSS 맥락에서 CI 파이프라인 실행을 고려했습니다.
- 기존 데이터가 예상보다 다양할 수 있음을 고려했습니다. 예를 들어, 새로운 모델 유효성 검사가 기존 레코드를 손상시킬 수 있습니다. 기존 데이터가 유효성 검사를 통과할 것이라고 확인하지 않은 경우, 기존 데이터에 대한 유효성을 선택 사항으로 만드는 것을 고려하세요.
- 경고와 함께 테스트가 통과하고 실패한 작업에
Flaky test '<path/to/test>' was found in the list of files changed by this MR.
라는 텍스트가 포함되어 있는 경우, 이 테스트를 수정했거나 이 불안정한 테스트를 무시할 수 있는 이유를 설명하는 증거를 제공했습니다.
성능, 신뢰성 및 가용성
- 이 MR이 성능에 해롭지 않다는 확신이 있거나, 성능 영향을 평가하는 데 도움이 될 리뷰어에게 요청했습니다. (병합 요청 성능 가이드라인)
- MR 설명에 대한 데이터베이스 리뷰어 정보를 추가했습니다 또는 필요 없다고 판단했습니다.
- 이 변경의 가용성과 신뢰성 위험을 고려했습니다.
- 미래 예상 성장을 기반으로 스케일링 위험을 고려했습니다.
- 이 변경이 평균 고객보다 더 많은 데이터를 보유할 수 있는 대규모 고객에게 미치는 성능, 신뢰성 및 가용성 영향을 고려했습니다.
- 최소 시스템에서 GitLab을 실행할 수 있는 고객에게 이 변경이 미치는 성능, 신뢰성 및 가용성 영향을 고려했습니다.
가시성 도구
- 디버깅과 능동적인 성능 개선을 위한 충분한 도구를 포함했습니다. 기능 플래그, 로깅 및 도구 추가의 예시를 참조하세요.
문서화
- 변경 로그 트레일러를 포함했거나 필요 없다고 판단했습니다.
- 문서를 추가/업데이트했거나 이 MR에 대한 문서 변경이 필요 없다고 판단했습니다.
보안
- 이 MR이 자격 증명이나 토큰, 인증 및 인가 방법 처리 또는 저장에 대한 변경을 포함하는 경우, 보안 검토 가이드라인에 설명된 기타 항목에 대해,
~security
레이블을 추가하고@gitlab-com/gl-security/appsec
를 언급했습니다. - 언제 및 어떻게 보안 검토를 요청하는지에 대한 내부 애플리케이션 보안 검토 문서를 검토하고, 이 변경에 대해 보안 검토가 필요한 경우 요청했습니다.
- MR을 차단하는 보안 스캔 결과가 있는 경우( 병합 요청 승인 정책):
- 진짜 긍정적 결과에 대해서는 병합 요청이 병합되기 전에 수정해야 합니다. 이렇게 하면 병합 요청 승인 정책에서 요구하는 AppSec 승인을 제거할 수 있습니다.
- 잘못된 긍정적 결과에 대해서는 위험 수용에 대해 논의해야 할 사항이거나 의문이 있는 경우
@gitlab-com/gl-security/appsec
에 문의하십시오.
배포
- 이 변경이 높은 위험일 수 있으므로 기능 플래그 사용을 고려했습니다.
- 기능 플래그를 사용하는 경우, 프로덕션에서 테스트하기 전에 스테이징에서 변경 사항을 테스트할 계획이며, 모든 고객에게 롤아웃하기 전에 프로덕션 고객의 일부에게 먼저 롤아웃하는 것을 고려했습니다.
- 기본 설정 또는 새로운 설정 변경에 대해 인프라 부서에 완료 정의를 통지했거나 필요 없다고 판단했습니다.
준수 사항
- 올바른 MR 유형 레이블이 적용되었음을 확인했습니다.
병합 요청 작성자의 책임
최고의 솔루션을 찾고 이를 구현하는 책임은 병합 요청 작성자에게 있습니다.
작성자 또는 직접 책임 있는 개인 (DRI)은 코드 리뷰 수명 주기 동안 병합 요청에 할당된 사람으로 남아 있습니다. 자신을 할당자로 설정할 수 없는 경우, 검토자에게 이를 수행해 달라고 요청하십시오.
유지 관리자로부터 승인을 요청하기 전에, 그들은 다음과 같은 확신을 가져야 합니다:
- 실제로 해결하려던 문제를 해결하고 있습니다.
- 가장 적절한 방식으로 그렇게 합니다.
- 모든 요구 사항을 충족합니다.
- 남은 버그, 논리적 문제, 커버되지 않은 엣지 케이스 또는 알려진 취약점이 없습니다.
이를 수행하는 최선의 방법은 불필요한 일과를 피하기 위해 자신의 병합 요청에 대한 자가 리뷰를 수행하는 것입니다.
이를 위해 코드 리뷰 지침을 따르십시오. 이러한 자가 리뷰 중에 결정이나 트레이드 오프가 이루어진 줄이나 맥락 설명이 검토자가 코드를 더 쉽게 이해하는 데 도움이 될 수 있는 곳에 MR에 주석을 포함하십시오.
필요한 수준의 확신에 도달하기 위해 작성자는 조사 및 구현 과정에 적절하게 다른 사람들을 참여시키는 것이 기대됩니다.
그들은 도메인 전문가에게 다양한 솔루션에 대해 논의하거나 구현을 검토해 달라고 요청하고, 제품 관리자 및 UX 디자이너에게 혼란을 풀거나 최종 결과가 그들이 의도한 것과 일치하는지 확인하도록 권장됩니다. 데이터 모델 또는 특정 쿼리에 대한 입력을 얻기 위해 데이터베이스 전문가에게 문의하거나 다른 개발자에게 솔루션의 심층 리뷰를 요청할 수 있습니다.
기능을 전달하기 위해 많은 병합 요청이 필요할 것이라고 생각되면 (예: 개념 증명을 만들었고 기능이 10개 이상의 병합 요청으로 구성될 것이 확실한 경우), 기능에 대한 필수 이해가 있는 검토자 및 유지 관리자를 식별하는 것을 고려하십시오 (그들과 맥락을 공유합니다). 그런 다음 모든 병합 요청을 이러한 검토자에게 직접 전달하십시오.
여러 병합 요청에 대해 안정적인 검토자 상대를 두는 것이 효율성을 향상시킵니다.
병합 요청이 두 개 이상의 도메인을 다루는 경우 (예: 동적 분석 및 GraphQL), 각 도메인의 전문가에게 리뷰를 요청하십시오.
작성자가 병합 요청이 도메인 전문가의 의견이 필요한지 확신이 없으면, 이는 필요하다는 것을 나타냅니다. 없이는 그들의 솔루션에 대한 필요한 수준의 확신을 가지기 어려울 것입니다.
리뷰 전에 작성자는 병합 요청 차이에 댓글을 제출하여 검토자에게 중요 사항 및 추가 설명이나 주의가 필요한 내용을 경고하도록 요청합니다. 댓글을 달아야 할 수 있는 내용의 예시는 다음과 같습니다:
- 린트 규칙(RuboCop, JS 등)의 추가.
- 라이브러리(Ruby gem, JS lib 등)의 추가.
- 명확하지 않은 경우, 상위 클래스 또는 메소드에 대한 링크.
- 변경을 보완하기 위해 수행된 벤치마킹.
- 잠재적으로 안전하지 않은 코드.
검토자가 솔루션을 검증하기 위해 필요한 프로젝트, 스니펫 또는 기타 자산이 있는 경우, 리뷰 요청 전에 이러한 자산에 대한 액세스를 보장해야 합니다.
검토자를 할당할 때는 다음이 도움이 될 수 있습니다:
- 해당 검토자로부터 원하는 유형의 검토를 명시하는 주석을 MR에 추가합니다.
- 예를 들어, MR이 데이터베이스 쿼리를 변경하고 백엔드 코드를 업데이트하는 경우, MR 작성자는 먼저
~backend
리뷰와~database
리뷰가 필요합니다. 검토자를 할당하는 동안 작성자는 각 검토자에게 어떤 도메인을 검토해야 하는지를 알리는 주석을 추가합니다. - 많은 GitLab 팀원들이 여러 분야의 도메인 전문가이므로, 이러한 유형의 주석이 없으면 그들이 제공해야 하는 검토 유형이 모호할 수 있습니다.
- MR 리뷰 유형에 대한 명시는 MR 작성자에게 효율적이며, 작성자가 원하는 검토 유형을 받게 되어 효율적이고 MR 검토자도 즉시 어떤 유형의 검토를 제공해야 하는지 알 수 있습니다.
- 예제 1
- 예제 2
- 예를 들어, MR이 데이터베이스 쿼리를 변경하고 백엔드 코드를 업데이트하는 경우, MR 작성자는 먼저
피해야 할 사항:
- 검토자가 필요로 하지 않는 한, 직접 소스 코드에 TODO 주석을 추가하지 마십시오. TODO 주석이 행동 가능한 작업으로 추가된 경우, 관련 이슈에 대한 링크를 포함하십시오.
- 코드가 무엇을 하고 있는지를 설명하는 주석을 추가하지 마십시오. 비TODO 주석이 추가되는 경우, 그들은 무엇이 아닌 왜를 설명해야 합니다.
- 실패한 테스트가 있는 병합 요청에 대해 유지 관리자 리뷰를 요청하지 마십시오. 테스트가 실패하는 경우 리뷰를 요청해야 할 경우 설명과 함께 주석을 남기는 것이 중요합니다.
- 유지 관리자가 슬랙으로 접근할 수 있는 경우 이메일이나 슬랙을 통해 유지 관리자에게 과도하게 언급하지 마십시오. 병합 요청에 검토자를 추가할 수 없는 경우, 주석에서 유지 관리자를
@
언급하는 것이 허용되며 다른 모든 경우 검토자를 추가하는 것으로 충분합니다.
이는 리뷰어의 시간을 절약하고 작성자가 실수를 조기에 발견하는 데 도움이 됩니다.
리뷰어의 책임
리뷰어는 선택한 솔루션의 세부사항을 검토할 책임이 있습니다.
할당된 병합 요청을 검토 응답 SLO 내에 검토할 수 없는 경우:
-
저자에게 이용할 수 없음을 알립니다.
-
GitLab 리뷰 작업량 대시보드를 사용하여 새로운 리뷰어를 선택합니다.
-
새로운 리뷰어를 병합 요청에 할당합니다.
이는 행동 편향을 나타내며 효율적인 MR 검토 진행을 보장합니다.
다음과 같은 댓글을 추가하세요:
안녕하세요 <@mr-author>, 리뷰를 할 수 없지만 이 프로젝트를 위해 [룰렛 휠을 돌렸습니다](https://gitlab-org.gitlab.io/gitlab-roulette/) 그리고 <@new-reviewer>가 선택되었습니다.
@new-reviewer, 시간이 되실 때 이 MR을 검토해 주실 수 있나요? 만약 불가능하시다면 [룰렛 휠을 다시 돌려서](https://gitlab-org.gitlab.io/gitlab-roulette/) 새로운 리뷰어를 선택하고 할당해 주세요, 감사합니다.
/assign_reviewer <@new-reviewer>
/unassign_reviewer me
병합 요청 검토하기를 철저히 수행하세요.
병합 요청이 모든 기여 수락 기준을 충족하는지 확인하세요.
일부 병합 요청은 세부사항을 돕기 위해 도메인 전문가가 필요할 수 있습니다.
리뷰어는 해당 분야의 도메인 전문가가 아닌 경우 다음과 같은 작업을 수행할 수 있습니다:
- 병합 요청을 검토하고 다른 리뷰어에게 중복 검토를 요청합니다. 이 전문가는 다른 리뷰어이거나 유지 관리자가 될 수 있습니다.
- 자신이 더 적합하다고 생각하는 다른 리뷰어에게 검토를 전달합니다.
- 도메인 전문가가 없으면 최선의 노력에 따라 검토합니다.
병합 요청이 다음과 같을 경우 저자가 병합 요청을 더 작은 병합 요청으로 분할하도록 안내해야 합니다:
- 너무 큼.
- 한 가지 이상의 문제를 수정함.
- 한 가지 이상의 기능을 구현함.
- 추가 위험을 초래하는 높은 복잡성을 가짐.
저자는 현재 유지 관리자가 분할 MRs을 검토하도록 요청하거나 다른 그룹의 유지 관리자를 요청할 수 있습니다.
모든 요구 사항을 충족한다고 확신할 경우 다음을 수행해야 합니다:
-
승인을 선택합니다.
-
@
저자에게 언급하여 할 일 알림을 생성하고 병합 요청이 검토되고 승인되었다고 조언합니다. -
유지 관리자로부터 검토를 요청합니다. 일반적으로 도메인 전문가의 요청으로 기본 설정되어 있지만, 해당 전문가가 없거나 병합 요청이 도메인 전문가의 검토가 필요하지 않다고 생각되면, 리뷰어 룰렛 제안을 따르셔도 됩니다.
-
자신을 리뷰어에서 제거합니다.
유지 관리자의 책임
유지 관리자는 GitLab 코드베이스의 전반적인 건강, 품질 및 일관성에 대한 책임이 있습니다.
따라서 그들의 리뷰는 전반적인 아키텍처, 코드 조직, 관심사의 분리를 비롯한 것들에 주로 초점을 맞춥니다.
유지 관리자의 작업은 전체 GitLab 코드베이스에 대한 지식에만 의존하고 특정 도메인에 의존하지 않기 때문에, 그들은 모든 팀과 모든 제품 영역에서 병합 요청을 검토, 승인 및 병합할 수 있습니다.
유지 관리자는 병합 요청이 수락 기준을 합리적으로 충족하는 것을 보장하는 DRI입니다.
일반적으로 품질은 모두의 책임이지만, MR의 유지 관리자는 이러한 일반 품질 기준을 충족하도록 보장하는 책임이 있습니다.
여기에는 후속 문제에서 기술 부채 생성 피하기가 포함됩니다.
유지 관리자가 만약 병합 요청이 상당하다고 생각하거나 도메인 전문가가 필요하다고 느끼면, 다른 리뷰어 또는 유지 관리자의 검토를 요청할 수 있는 재량을 가집니다. 다음은 유지 관리자가 리뷰 중에 이를 적극적으로 수행하는 몇 가지 예입니다:
-
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/82708#note_872325561
-
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38003#note_387981596
-
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/14017#note_178828088
유지 관리자는 병합하기 전에 선택한 솔루션의 세부사항도 검토하려고 최선을 다하지만, 그들이 반드시 도메인 전문가가 아니기 때문에, 불합리한 시간 투자의 투자 없이 그렇게 하는 데에 어려움을 느낄 수 있습니다. 이러한 경우, 그들은 저자와 이전 리뷰어의 판단에 위임하고 자신의 주요 책임에 집중합니다.
리뷰어로 참여한 개발자가 유지 관리자인 경우, 그들이 최종적으로 승인하고 병합할 유지 관리자로 선택되지 않는 것이 좋습니다.
유지 관리자는 병합하기 전에 병합 요청이 필요한 승인자로부터 승인을 받았는지 확인해야 합니다. 여전히 다른 사람의 추가 승인을 기다리고 있다면, 자신을 리뷰어에서 제거한 다음 @
저자에게 언급하고 댓글로 그 이유를 설명하세요. 코드를 병합할 때는 리뷰어로 유지하세요.
특정 병합 요청이 안정적인 브랜치를 대상으로 할 수 있습니다. 이러한 요청을 처리하는 방법에 대한 개요는 패치 릴리즈 매뉴얼을 참조하세요.
병합 후, 유지 관리자는 병합 요청에 나열된 리뷰어로 남아야 합니다.
리뷰어 기능의 단백질
2021년 3월 18일, 리뷰어 기능을 효율적이고 일관되게 활용하기 위한 업데이트된 프로세스가 시행되었습니다.
여기 변경 사항의 요약이 있으며, 위 섹션에도 반영되어 있습니다.
- 병합 요청 작성자 및 DRI는 담당자로 남습니다.
- 작성자는 사용자를 리뷰어로 지정하여 리뷰를 요청합니다.
- 리뷰어는 리뷰 및 승인이 완료되면 스스로 할당 해제합니다.
- 마지막 승인자(병합 MR)의 경우 리뷰어로 할당된 상태입니다.
모범 사례
모든 사람
- 친절하십시오.
- 많은 프로그래밍 결정이 의견이라는 것을 받아들이십시오. 트레이드오프를 논의하고, 선호하는 것을 공유하며, 신속하게 해결책에 도달하십시오.
- 질문하십시오; 요구하지 마십시오. (“이름을
:user_id
로 짓는 것에 대해 어떻게 생각하십니까?”) - 명확한 설명을 요청하십시오. (“무슨 뜻인지 이해하지 못했습니다. 설명해 주실 수 있나요?”)
- 코드에 대한 선택적 소유권을 피하십시오. (“내 것”, “내 것이 아님”, “당신의 것”)
- 개인적 특성을 언급하는 것으로 보일 수 있는 용어 사용을 피하십시오. (“멍청한”, “어리석은”). 모두가 지능적이고 의도적으로 행동한다고 가정하십시오.
- 명확하게 하십시오. 사람들이 온라인에서 당신의 의도를 항상 이해하지 못한다는 것을 기억하십시오.
- 겸손하십시오. (“확실하지 않습니다 - 찾아봅시다.”)
- 과장을 사용하지 마십시오. (“항상”, “절대”, “끝없이”, “아무것도”)
- 비꼬기 사용에 주의하십시오. 우리가 하는 모든 것은 공개적입니다; 당신과 오랜 동료에게는 좋은 성격의 농담처럼 보이는 것이 프로젝트에 새로운 사람에게는 매정하고 환영받지 못하는 것으로 비칠 수 있습니다.
- “이해하지 못했다”거나 “대안 제안:”과 같은 코멘트가 너무 많다면 일대일 채팅이나 화상 통화를 고려하세요. 일대일 논의를 요약한 후속 코멘트를 게시하십시오.
- 특정 인물에게 질문을 할 경우 항상 그들을 언급하여 코멘트를 시작하십시오; 이렇게 하면 알림 수준이 “언급됨”으로 설정된 경우 그들이 확인할 수 있으며, 다른 사람들이 응답할 필요가 없음을 이해하게 됩니다.
병합 요청을 리뷰받기
코드 리뷰는 여러 번의 반복을 거칠 수 있는 프로세스라는 점을 염두에 두십시오. 리뷰어는 처음에는 보지 못했을 수도 있는 사항을 나중에 발견할 수 있습니다.
- 코드의 첫 번째 리뷰어는 _당신_입니다. 새 브랜치의 첫 푸시를 수행하기 전에 전체 diff를 읽어보십시오. 그것이 이해가 가나요? 전체 변경 목적과 관련 없는 것을 포함했나요? 디버깅 코드를 제거하는 것을 잊은 적은 있나요?
- 병합 요청 가이드라인에 설명된 대로 상세한 설명을 작성하십시오. 일부 리뷰어는 제품 기능 또는 코드베이스의 특정 영역에 익숙하지 않을 수 있습니다. 철저한 설명은 모든 리뷰어가 귀하의 요청을 이해하고 효과적으로 테스트하는 데 도움이 됩니다.
- 귀하의 변경 사항이 먼저 병합되는 다른 것에 의존하고 있다면, 설명에 이를 기록하고 병합 요청 종속성을 설정하십시오.
- 리뷰어의 제안에 감사하십시오. (“좋은 판단입니다. 그 변경을 하겠습니다.”)
- 개인적으로 받아들이지 마십시오. 리뷰는 코드에 대한 것이지 당신에 대한 것이 아닙니다.
- 코드가 존재하는 이유를 설명하십시오. (“이것은 이러한 이유 때문에 그렇게 되어 있습니다. 이 클래스/파일/메서드/변수를 이름을 바꾸면 더 명확할까요?”)
- 관련 없는 변경 사항 및 리팩토링은 미래의 병합 요청/문제로 분리하십시오.
- 리뷰어의 관점을 이해하려고 노력하십시오.
- 모든 코멘트에 응답하려고 노력하십시오.
- 병합 요청 작성자는 자신이 완전히 처리한 스레드만 해결합니다. 열린 응답, 열린 스레드, 제안, 질문 또는 기타 사항이 있는 경우, 해당 스레드는 리뷰어가 해결해야 합니다.
- 모든 피드백이 MR이 병합되기 전에 그들이 추천한 변경 사항을 반드시 포함해야 하는 것은 아닙니다. 이것이 필요한지, 아니면 이 피드백을 향후 처리하기 위한 후속 문제를 생성해야 하는지는 MR 작성자와 리뷰어의 판단입니다.
- 이전 피드백에 기반하여 커밋을 개별 커밋으로 분리하여 브랜치에 푸시하십시오. 브랜치가 병합 준비가 될 때까지 스쿼시하지 마십시오. 리뷰어는 이전 피드백을 기반으로 한 개별 업데이트를 읽을 수 있어야 합니다.
- 또 다른 리뷰 라운드를 받을 준비가 되면 리뷰어에게 새 리뷰를 요청하십시오. 리뷰를 요청할 수 있는 능력이 없다면 대신
@
로 리뷰어를 언급하십시오.
리뷰 요청하기
병합 요청을 검토할 준비가 되었으면, 초기 리뷰 요청하기를 선택하여 승인 지침을 기반으로 리뷰어를 선택해야 합니다.
병합 요청에 여러 검토 영역이 있는 경우, 리뷰어가 어떤 영역을 검토해야 하고 어느 단계(첫 번째 또는 두 번째)에서 요청해야 하는지를 명시하는 것이 좋습니다.
이렇게 하면 여러 영역의 리뷰어로 자격이 있는 팀원이 어떤 영역을 검토해야 하는지 알 수 있습니다.
예를 들어, 병합 요청에 backend
와 frontend
문제가 모두 있는 경우, 리뷰어를 다음과 같은 방식으로 언급할 수 있습니다:
@john_doe, ~backend 검토해 주실 수 있나요?
또는 @jane_doe - 이 MR에 대해 ~frontend 유지 관리자의 검토를 해주실 수 있나요?
또한 workflow::ready for review
레이블을 사용할 수 있습니다. 이는 해당 병합 요청이 검토할 준비가 되었음을 의미하며, 어떤 리뷰어든 선택할 수 있습니다. 시간 압박이 없을 경우에만 해당 레이블을 사용하는 것이 권장되며, 병합 요청이 리뷰어에게 지정되어 있는지 확인해야 합니다.
병합 요청이 첫 번째 리뷰어로부터 승인을 받으면 유지 관리자로 전달될 수 있습니다. 도메인 전문성을 갖춘 유지 관리자를 선택하는 것이 기본이며, 그렇지 않은 경우 리뷰어 룰렛 추천을 따르거나 ready for merge
레이블을 사용하세요.
때로는 유지 관리자가 검토를 위해 사용할 수 없을 수 있습니다. 사무실에 없거나 용량 초과일 수 있습니다.
유지 관리자의 프로필에서 그들의 가용성을 확인할 수 있고, 확인해야 합니다. 룰렛에서 추천된 유지 관리자가 사용할 수 없는 경우 해당 목록에서 다른 사람을 선택하세요.
병합 요청이 검토되도록 하는 것은 작성자의 책임입니다. ready for review
상태에 너무 오랫동안 머물 경우, 특정 리뷰어에게 리뷰 요청을 하는 것이 좋습니다.
리뷰 자원봉사하기
용량이 있는 GitLab 엔지니어는 정기적으로 리뷰할 병합 요청 목록을 확인하고, 리뷰하고 싶은 병합 요청의 리뷰어로 자신을 추가할 수 있습니다.
병합 요청 검토하기
변경이 왜 필요한지 이해하세요(버그 수정, 사용자 경험 개선, 기존 코드 리팩토링 등). 그런 다음:
- 반복의 수를 줄이기 위해 검토를 철저히 하십시오.
- 당신이 강하게 느끼는 아이디어와 그렇지 않은 아이디어를 소통하세요.
- 문제를 해결하면서 코드를 단순화할 방법을 찾으세요.
- 대안 구현을 제안하세요. 그러나 저자가 이미 그것들을 고려했을 것이라고 가정하세요. (“여기에서 사용자 정의 유효성 검사를 사용하는 것에 대해 어떻게 생각하시나요?”)
- 저자의 관점을 이해하려고 노력하세요.
- 브랜치를 확인하고, 변경 내용을 로컬에서 테스트하세요. 수동 테스트를 얼마나 수행할지는 결정할 수 있습니다. 당신의 테스트는 자동화된 테스트를 추가할 기회를 가져올 수 있습니다.
- 코드의 일부를 이해하지 못한다면, 그렇게 말하세요. 다른 누군가도 혼란스러울 가능성이 높습니다.
- 작성자가 제안을 해결/처리하기 위해 무엇이 필요한지 명확히 알 수 있도록 하세요.
- 의도를 전달하기 위해 전통적 댓글 형식을 사용하는 것을 고려하세요.
- 비의무적인 제안의 경우, 작성자가 병합 요청 내에서 선택적으로 해결할 수 있음을 알 수 있도록 (비차단)으로 장식하세요. 비의무적인 제안만 있는 경우, 비동기 사이클을 줄이기 위해 MR을 다음 단계로 이동하십시오. 첫 번째 라운드 검토자일 때는 유지 관리자가 검토할 수 있도록 전달하세요. 최종 승인 유지 관리자인 경우, 비차단 제안으로 후속 작업을 생성하고 병합하거나 자동 병합을 설정하세요. 작성자는 비차단 제안을 구현하여 자동 병합을 취소하거나 MR이 병합된 후 후속 MR을 제공하거나 제안서를 구현하지 않기로 결정할 수 있습니다.
- Chrome/Firefox 추가 기능을 사용하여 전통적 댓글 접두사를 적용할 수 있습니다.
- 열린 종속성이 없는지 확인하세요. 차단 여부를 확인하기 위해 연결된 문제를 확인하세요. 필요시 저자와 명확히 하세요. 하나 이상의 열린 MR로 차단된 경우, MR 종속성을 설정하세요.
-
한 라운드의 라인 노트 이후에는 다음과 같은 요약 노트를 게시하는 것이 유용할 수 있습니다: “좋아 보입니다”, 또는 “처리할 몇 가지가 있습니다.”
- 리뷰 후 변경이 필요한 경우 작성자에게 알려주세요.
경고: 병합 요청이 포크에서 온 것이라면, 커뮤니티 기여를 위한 추가 지침도 확인하세요.
병합 요청 병합하기
병합 결정을 내리기 전에:
- 마일스톤을 설정하세요.
- 올바른 MR 유형 레이블이 적용되었는지 확인하세요.
- danger bot, 코드 품질 및 기타 보고서의 경고 및 오류를 고려하세요.
위반에 대한 강력한 주장이 제기되지 않는 한, 병합하기 전에 이러한 문제는 해결되어야 합니다.
MR이 실패한 작업으로 병합된 경우 주석을 남겨야 합니다. - MR에 품질 관련 변경 사항과 비품질 관련 변경 사항이 모두 포함된 경우,
품질 관련 변경 사항이 소프트웨어 엔지니어에 의해 승인된 후 사용자에게 영향을 주는 변경 사항(백엔드, 프론트엔드 또는 데이터베이스)에 대한 관련 유지 관리자가 MR을 병합해야 합니다.
최소한 한 명의 유지 관리자가 MR을 승인해야 병합할 수 있습니다. MR 작성자와
MR에 커밋을 추가하는 사람은 MR을 승인할 권한이 없으며, MR에 기여하지 않은 유지 관리자의 승인을 받아야 합니다.
일반적으로 최종 승인자는 MR을 병합해야 합니다.
최종 승인이 MR을 병합하지 않을 수 있는 시나리오:
- 승인자가 승인 후 자동 병합을 설정하는 것을 잊음.
- 승인자가 자신이 최종 승인자라는 것을 인식하지 못함.
- 승인자가 자동 병합을 설정했지만 GitLab에 의해 해제됨.
이러한 시나리오 중 하나가 발생할 경우, MR 작성자는 모든 필요한 승인이 있고
저장소에 대한 병합 권한이 있는 경우 자신의 MR을 병합할 수 있습니다.
이는 GitLab의 행동 편향 가치와 일치합니다.
이 정책은 GitLab의
변경 관리 통제의 CHG-04 통제를 충족하기 위해 마련되었습니다.
gitlab-org/gitlab
에서 이 정책을 구현하기 위해 MRs가
최상위 CODEOWNERS 유지 관리자의 승인을 받도록 다음 설정이 활성화되었습니다:
gitlab-org/gitlab
의 CODEOWNERS
파일을 업데이트하려면
코드 소유자 승인 핸드북 섹션에서 설명한 절차를 따르세요.
로컬에서 리베이스하거나 제안을 적용하는 것과 같은 일부 작업은
커밋을 추가하는 것과 동일한 것으로 간주되며 기존 승인을 재설정할 수 있습니다.
UI에서 리베이스하거나 /rebase
빠른 작업으로 리베이스할 때는 승인이 제거되지 않습니다.
병합 준비가 완료되면:
경고: 포크에서 병합 요청이 오는 경우, 커뮤니티 기여를 위한 추가 지침도 확인하세요.
-
병합 요청에 커밋이 많이 있을 경우 Squash and merge 기능을 사용하는 것을 고려하세요.
코드를 병합할 때, 유지 관리자는
작성자가 이미 이 옵션을 설정했거나 병합 요청에 명백하게 엉망인 커밋 기록이 포함된 경우에만 스쿼시 기능을 사용해야 합니다.
그렇지 않으면 MR에 커밋이 몇 개만 있는 경우,
우리는 작성자의 설정을 존중하여 커밋을 스쿼시하지 않을 것입니다. - 병합 요청의 Pipelines 탭으로 이동하여 Run pipeline을 선택하세요.
그런 다음 Overview 탭에서 Auto-merge를 활성화하세요.
다음 정보를 고려하세요:-
기본 브랜치가 손상된 경우,
병합 요청을 병합하지 마세요
매우 특정한 경우를 제외하고는
다른 경우에는 다음 핸드북 지침을 따르세요. - 최신 파이프라인이 병합 요청이 승인되기 전에 생성된 경우,
전체 RSpec 스위트가 실행되었는지 확인하기 위해 새로운 파이프라인을 시작하세요.
MR에 백엔드 변경 사항이 포함되지 않은 경우에만 이 단계를 생략할 수 있습니다. -
최신 병합 결과 파이프라인이 8시간 전에 생성된 경우(안정적인 브랜치의 경우 72시간),
병합 요청이 대상 브랜치에 충분히 가까운 경우 새 파이프라인을 시작하지 않고 병합할 수 있습니다.
-
기본 브랜치가 손상된 경우,
-
MR을 자동 병합으로 설정하면 이후에 발견될 것으로 예상되는 사항에 대한
수정 사항을 처리해야 합니다. -
Squash and merge가 설정된 병합 요청의 경우,
스쿼시된 커밋의 기본 커밋 메시지는 병합 요청 제목에서 가져옵니다.
병합 전에 더 유익한 커밋 메시지가 있는 커밋을 선택하는 것을 권장합니다.
병합 결과 파이프라인 덕분에 작성자는
브랜치를 자주 리베이스할 필요가 없어졌습니다(충돌이 있는 경우에만).
병합 결과 파이프라인은 이미 최신 변경 사항을 main
에서 통합하고 있습니다.
이로 인해 검토/병합 사이클이 빨라졌으며, 유지 관리자는
최종 리베이스 요청을 하지 않아도 됩니다: 대신 MR 파이프라인을 시작하고 자동 병합을 설정하기만 하면 됩니다.
이 단계는 파이프라인 생성 시 최신 main
에 대한
병합 결과를 테스트하여 실제 Merge Trains 기능에 근접하게 만듭니다.
커뮤니티 기여
경고:
모든 변경 사항을 악성 코드에 대해 철저히 검토한 후
병합 결과 파이프라인을 시작하세요.
넓은 커뮤니티 기여자가 추가한 병합 요청을 검토할 때:
- 새로운 의존성과 의존성 업데이트(예: Ruby gems 및 Node 패키지)에 특히 주의하세요.
Gemfile.lock
또는yarn.lock
과 같은 파일의 변경은 사소해 보일 수 있지만,
악성 패키지가 가져오는 원인이 될 수 있습니다. - 링크와 이미지, 특히 문서 MRs를 검토하세요.
- 의심스러운 경우, 병합 요청을 수동으로 시작하기 전에
@gitlab-com/gl-security/appsec
의 누군가에게 병합 요청을 검토해 달라고 요청하세요. - 병합 요청이 현재 마일스톤에 포함될 가능성이 있는 경우에만 마일스톤을 설정하세요.
이는 병합 시점을 혼동하지 않도록 하고, 준비가 되지 않은 경우
마일스톤을 너무 자주 이동하지 않도록 합니다.
커뮤니티 병합 요청 인수
MR이 추가적인 변경이 필요하지만 작성자가 오랜 기간 동안 응답하지 않거나
MR을 완료할 수 없는 경우, GitLab이 이를 인수할 수 있습니다.
GitLab 엔지니어( 일반적으로 병합 요청 코치)가 다음을 수행합니다:
- 병합 요청에 댓글을 달아 병합될 수 있도록 인수할 것이라고 말합니다.
- 자신의 MR에
~"coach will finish"
레이블을 추가합니다. - 메인 브랜치에서 새로운 기능 브랜치를 생성합니다.
- 그들의 브랜치를 자신의 새로운 기능 브랜치에 병합합니다.
- 자신의 기능 브랜치를 메인 브랜치에 병합하기 위한 새로운 병합 요청을 엽니다.
- 자신의 MR에서 커뮤니티 MR을 링크하고
~"Community contribution"
으로 레이블을 추가합니다. - 필요한 최종 조정을 하고, 기여자에게 알리며
자신의 변경 사항을 검토할 기회를 제공합니다. - 내용이 모든 병합 요청 가이드라인을 준수하는지 확인합니다.
- 다른 병합 요청과 동일한 정기 검토 프로세스를 따릅니다.
올바른 균형
코드 리뷰에서 가장 어려운 것 중 하나는
작성자가 만든 코드에 대해 검토자가 얼마나 깊이 개입할 수 있는지의 올바른 균형을 찾는 것입니다.
- 올바른 균형을 찾는 방법을 배우는 데는 시간이 필요합니다.
그래서 우리는 병합 요청을 검토하는 데 일정 시간을 보낸 후
유지 관리자가 되는 검토자를 두고 있습니다. - 버그를 찾는 것이 중요하지만, 좋은 디자인에 대한 생각도 중요합니다.
추상화를 구축하고 좋은 디자인을 만드는 것은
복잡성을 숨기고 향후 변경을 용이하게 만듭니다. -
코드 스타일을 enforcing하고 개선하는 것은
주로 자동화를 통해 이루어져야 합니다. - 작성자에게 디자인 변경을 요청하는 것은
기여된 코드의 전체 재작성 의미할 수 있습니다.
보통은 이를 하기 전에 다른 유지 관리자나 검토자에게 문의하는 것이 좋지만,
중요하다고 믿으면 용기 있게 진행하세요. -
Iteration의 이익을 위해,
검토 제안이 차단되지 않는 변경 또는 개인적인 선호(문서화되거나 합의된 요구 사항 아님)인 경우
병합 요청을 작성자에게 되돌려주기 전에
승인하는 것을 고려하세요.
이는 그들이 동의할 경우 제안을 구현할 수 있게 하거나,
즉시 검토를 위해 유지 관리자에게 전달할 수 있게 합니다.
이는 우리의 전반적인 병합 소요 시간을 단축하는 데 도움이 될 수 있습니다. - 올바르게 실행하는 것과 즉시 실행하는 것의 차이가 있습니다.
이상적으로 우리는 전자를 해야 하지만, 현실 세계에서는 후자도 필요합니다.
좋은 예는 가능한 한 빨리 릴리즈되어야 하는 보안 수정입니다.
긴급 수정인 병합 요청에서 작성자에게
주요 리팩토링을 요청하는 것은 피해야 합니다. - 오늘 잘 실행하는 것이 내일 완벽하게 실행하는 것보다
보통 더 좋습니다. 오늘 한 가지 이상한 방식으로 배송하는 것은
보통 내일 잘 수행하는 것보다 나쁜 경우가 많습니다.
올바른 균형을 찾을 수 없으면 다른 사람에게 의견을 물어보세요.
GitLab 전용 문제
GitLab은 많은 장소에서 사용됩니다. 많은 사용자가 우리의 Omnibus 패키지를 사용하지만,
일부는 Docker 이미지를 사용하고, 일부는 소스에서 설치하며,
다른 설치 방법도 사용할 수 있습니다. GitLab.com 자체는 큰 Enterprise Edition 인스턴스입니다. 이는 몇 가지 의미가 있습니다:
-
쿼리 변경 사항은 GitLab.com의 규모에서 성능 저하가 발생하지 않도록 테스트해야 합니다:
-
로컬에서 대량의 데이터를 생성하는 것이 도움이 될 수 있습니다.
-
GitLab.com에서 쿼리 계획을 요청하는 것이 이를 검증하는 가장 신뢰할 수 있는 방법입니다.
-
-
데이터베이스 마이그레이션은 다음과 같아야 합니다:
-
되돌릴 수 있어야 합니다.
-
GitLab.com의 규모에서 성능이 우수해야 하며, 확신이 없다면 유지 관리자가 스테이징 환경에서 마이그레이션을 테스트하도록 요청하세요.
-
올바르게 분류되어야 합니다:
-
정기 마이그레이션은 새 코드가 인스턴스에서 실행되기 전에 실행됩니다.
-
배포 후 마이그레이션은 새 코드가 배포된 후 인스턴스가 이를 수행하도록 구성될 때 실행됩니다.
-
배치 백그라운드 마이그레이션은 Sidekiq에서 실행되며,
GitLab.com 규모에서 배포 후 마이그레이션 시간 제한을 초과하는 마이그레이션에 사용해야 합니다.
-
-
-
Sidekiq 작업자는 역방향 호환되지 않는 방식으로 변경될 수 없습니다:
-
Sidekiq 큐는 배포가 발생하기 전에 비워지지 않으므로 이전 버전의 GitLab에서 큐에 작업자가 존재할 수 있습니다.
-
메서드 시그니처를 변경해야 하는 경우, 두 개의 릴리스에 걸쳐 변경하려고 시도하고, 그 첫 번째 릴리스에서 이전 및 새로운 인수를 모두 수용하세요.
-
마찬가지로, 작업자를 제거해야 하는 경우, 한 릴리스에서 작업 일정에서 중단하고 다음 릴리스에서 제거하세요. 이렇게 하면 기존 작업이 실행될 수 있습니다.
-
잊지 마세요, 모든 인스턴스가 모든 중간 버전으로 업그레이드되는 것은 아니므로(일부 사용자는 X.1.0에서 X.10.0으로 진행하거나 더 큰 업그레이드를 시도할 수 있습니다!), 저렴하게 수행할 수 있다면 이전 형식을 수용하는 데 관용을 베푸세요.
-
-
캐시된 값은 릴리스 간 지속될 수 있습니다. 캐시된 값이 반환하는 유형(예: 문자열 또는 nil에서 배열로 변경)을 변경해야 하는 경우, 동시에 캐시 키를 변경하세요.
-
설정은 마지막 수단으로 추가되어야 합니다. GitLab Rails에 새 설정 추가를 참조하세요.
-
파일 시스템 접근은 클라우드 네이티브 아키텍처에서 불가능합니다. 수행해야 할 파일 저장을 위해 객체 스토리지를 지원하는지 확인하세요. 더 자세한 내용은 업로드 문서를 참조하세요.
고객 중요 병합 요청
병합 요청은 비즈니스에 상당한 이점이 있기 때문에 고객 중요 우선 사항으로 고려되는 것이 유리할 수 있습니다.
고객 중요 병합 요청의 속성:
-
Development의 수석 이사 이상의 승인이 필요하며, 병합 요청이 고객 중요 사항으로 자격을 갖추었다고 승인해야 합니다. 또는, 그들의 직접 보고 2명이 승인하면 그 또한 승인으로 간주할 수 있습니다.
-
DRI는 병합 요청에
customer-critical-merge-request
레이블을 적용합니다. -
고객 중요 병합 요청에 관련된 리뷰어와 유지 관리자는 이 결정이 내려지자마자 참여해야 합니다.
-
고객 중요 병합 요청에 관련된 작업의 우선순위를 두어야 하며, 이를 위해 필요한 집중 시간을 확보해야 합니다.
-
고객 중요 병합 요청 작업 시 GitLab 가치와 프로세스를 준수해야 하며, 특히 가족 및 친구를 먼저 고려하고, 일이 완료되는 정의, 반복 및 준비가 되었을 때 배포에 유의해야 합니다.
-
고객 중요 병합 요청은 보안을 감소시키지 않으며, 데이터 손실 위험을 초래하거나 가용성을 저하시키거나 기존 기능을 중단시키지 않아야 합니다. 이는 기술적 결정 우선시기 프로세스에 따릅니다.
-
고객 중요 요청에서는 관련자들이 동기적으로(Zoom, Slack) 조정하는 것을 _권장_하며, 비동기적으로(병합 요청 주석) 조정하고 이것이 병합 소요 시간을 줄일 수 있을 것이라고 믿는다면 그렇게 하세요. 비록 이것이 효율성을 희생할 수도 있지만 말입니다.
-
고객 중요 병합 요청이 병합된 후, 미래의 고객 중요 병합 요청의 빈도를 줄이기 위한 목적으로 회고가 완료되어야 합니다.
예시
코드 리뷰가 어떻게 진행되는지는 새로운 기여자들에게 놀라움을 줄 수 있습니다. 여기 여러분이 기대할 수 있는 내용을 정리하는 데 도움이 될 몇 가지 코드 리뷰 예시가 있습니다.
“DiffNote
를 수정하여 디자인에 재사용”:
이 리뷰는 개행에 대한 사소한 지적부터 디자인 버전이 무엇인지, 특정 파일의 이전 버전이 없을 경우(부모 대 blank sha
대 빈 트리) 어떻게 비교해야 하는지에 대한 이유까지 모든 것을 포함했습니다.
“다중 행 제안 지원”:
MR 자체는 FE와 BE 간의 협업으로 구성되며, 리뷰어를 위한 저자의 주석 문서화도 포함됩니다. 사소한 지적, 정보에 대한 질문이 있으며, 마지막에는 보안 취약점이 언급되었습니다.
“프로젝트당 여러 개의 저장소 허용”:
ZJ는 이로 인해 영향을 받을 수 있는 다른 프로젝트(워크호스)에 대해 언급하며 일관성을 위한 몇 가지 개선 사항을 제안했습니다. 그리고 James의 코멘트는 전체적인 코드 품질(위임 사용, &.
같은 것들) 향상에 도움이 되었고, 코드를 보다 견고하게 만들었습니다.
“병합 요청을 위한 여러 담당자 지원”:
코드베이스의 여러 부분에 영향을 미치는 MR에 대한 협업의 좋은 예입니다. Nick은 흥미로운 모서리 사례를 지적했고, James Lopez 또한 가져오기/내보내기 기능에 대한 우려를 제기하며 참여했습니다.
크레딧
주로 thoughtbot
코드 리뷰 가이드를 기반으로 합니다.