코드 리뷰 가이드라인

이 가이드에는 코드 리뷰를 수행하고 코드를 검토하는 데 도움이 되는 조언과 모범 사례가 포함되어 있습니다.

GitLab CE 및 EE에 대한 모든 병합 요청은 GitLab 팀 구성원이 작성한 것이든, 보다 넓은 커뮤니티 구성원이 작성한 것이든, 코드가 효과적이고 이해하기 쉽고 유지 관리하기 좋으며 안전하도록 보장하기 위해 코드 리뷰 프로세스를 거쳐야 합니다.

병합 요청을 검토하고 승인하고 병합하는 방법

시작하기 전에:

코드를 검토할 즉시 리뷰어에 의해 코드가 검토되어야 합니다. 이 리뷰어는 귀하의 그룹이나 팀에서 올 수 있거나 도메인 전문가일 수 있습니다. 리뷰어는:

  • 선택한 솔루션과 구현에 대한 다른 의견을 줄 수 있습니다.
  • 버그, 논리 문제 또는 처리되지 않은 에지 케이스를 찾는 데 도움을 줄 수 있습니다.

병합 요청이 작고 검토하기 쉬운 경우, 리뷰어 단계를 건너뛰고 maintainer에게 직접 물어볼 수 있습니다.

어떤 것이 “작고 간단한지”에 대해서는 모호한 영역입니다. 여기 작고 간단한 변경의 예가 있습니다:

  • 오타 수정이나 작은 복사 변경(예시 참조).
  • 행동이나 데이터를 변경하지 않는 작은 리팩터링.
  • 1개월 이상 기본으로 활성화된 기능 플래그에 대한 참조 삭제.
  • 사용되지 않는 메서드 또는 클래스 제거.
  • 5개 이하의 코드 라인을 변경해야 하는 잘 이해되는 로직 변경.

그렇지 않으면, 병합 요청은 MR이 닿는 각 카테고리(예: 백엔드, 데이터베이스)에서 리뷰어에 의해 먼저 검토되어야 하며, 유지보수자는 관련 도메인 지식을 가지고 있지 않을 수 있습니다. 이는 작업 부하를 분산시키는 데도 도움이 됩니다.

보안 스캔이나 코멘트에 대한 지원이 필요한 경우, Application Security Team (@gitlab-com/gl-security/appsec)를 포함하세요.

리뷰어는 사이드바의 리뷰어 기능을 사용합니다. 리뷰어는 병합 요청 승인을 추가하여 승인을 추가할 수 있습니다.

병합 요청이 닿는 영역에 따라 1명 이상의 유지보수자에 의해 승인되어야 합니다. 승인 버튼은 병합 요청 위젯에 있습니다.

병합 요청을 병합하려면 또한 유지보수자가 필요합니다. 1개 이상의 승인이 필요한 경우, 마지막으로 리뷰하고 승인한 유지보수자가 병합합니다. Verify와 같은 일부 도메인 영역은 CODEOWNERS 규칙에 따라 도메인 전문가의 승인이 필요합니다. CODEOWNERS 섹션은 독립적인 승인 규칙이기 때문에 일부 규칙(예: Verify)은 더 일반적인 승인 규칙(예: 백엔드)의 하위 집합일 수 있습니다. 더 효율적인 프로세스를 위해 저자는 일반적인 승인 전에 도메인별 승인을 찾아야 합니다. 도메인별 승인자는 유지보수자가 될 수도 있으며, 그렇다면 도메인 특정 사항과 더 넓은 변경 사항을 동시에 검토하고 둘 다 승인해야 합니다.

저자 책임에 대해 자세히 알아보세요.

도메인 전문가

도메인 전문가는 특정 기술, 제품 기능 또는 코드베이스 영역에 탄탄한 경험을 가진 팀 구성원입니다. 팀 구성원은 도메인 전문가로 자체 식별하고, 팀 프로필에 추가하는 것이 권장됩니다.

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

다음과 같은 가정을 하고 있습니다:

  • 특정 스테이지/그룹(예: 생성: 소스 코드)에서 근무하는 팀 구성원은 자신이 근무하는 응용 프로그램 영역에 대한 도메인 전문가로 간주됩니다.
  • 특정 기능(예: 검색)에서 작업하는 팀 구성원은 해당 기능에 대한 도메인 전문가로 간주됩니다.

우리는 코드 리뷰를 위해 팀 구성원에게 도메인 지식을 가진 리뷰어로 할당하는 것을 기본으로 설정합니다. 디자인 리뷰는 사용자 경험 리뷰 룰렛의 권장 리뷰어를 기본으로 설정합니다. 디자이너 용량 한계로 인해 제품 디자이너가 지원하지 않는 영역은 커뮤니티 기여인 경우를 제외하고 사용자 경험 리뷰가 더 이상 필요하지 않습니다. 적절한 도메인 전문가가 없는 경우, 팀 구성원 중 어떤 사람이든 MR을 검토하거나 리뷰어 룰렛의 권장을 따를 수 있습니다(디자인 리뷰에 대한 자세한 내용은 위를 참조하세요).

도메인 전문가를 찾기 위해:

  • Merge Request 승인 위젯에서 검토자보를 선택하세요. 이 위젯은 코드베이스 영역별로 권장 및 필수 승인을 보여줍니다. 이러한 규칙은 소유자 코드에 정의되어 있습니다.
  • 스테이지 또는 그룹과 관련된 팀 구성원 목록을 확인하세요.
  • 엔지니어링 프로젝트 페이지나 GitLab 팀 페이지에서 팀 구성원의 도메인 전문성을 확인하세요. 도메인은 자체 식별되므로 귀하의 판단을 사용하여 MR 변경사항을 도메인에 매핑하세요.
  • MR에 기여한 파일을 확인한 팀 구성원을 찾으세요. git log <file>을 실행하여 로그를 확인할 수 있습니다.
  • 해당 파일을 검토한 팀 구성원을 찾으세요. 커밋 SHA를 사용하여 관련 병합 요청을 찾을 수 있습니다.
    1. git log <file>을 사용하여 커밋 SHA를 가져옵니다.
    2. <SHA>를 사용하여 관련 병합 요청을 선택합니다.
    3. 커밋에 표시된 관련 병합 요청을 선택합니다.

리뷰어 룰렛

참고: 리뷰어 룰렛은 GitLab.com에서 사용하기 위한 내부 도구로, 고객 설치에서는 사용할 수 없습니다.

참고: %16.11까지, GitLab은 실험을 진행하여 헝그리 지표와 바쁨 지표를 제거하고 있습니다.

위험 봇은 당신의 병합 요청이 영향을 줄 것으로 보이는 코드베이스의 각 영역에 대해 무작위로 리뷰어와 메인테이너를 선택합니다. 개발자 리뷰어를 위한 권장 사항을 제공하며, 누군가 다른 사람이 더 적합하다고 생각한다면 재정의해야 합니다.

승인 지침도메인 전문가를 선택하는 데 도움이 될 수 있습니다.

우리는 제품 디자이너를 포함하는 팀의 MR에 대해서만 UX 리뷰를 수행합니다. 이러한 팀에서의 사용자 대상 변경 사항은 기능 플래그 뒤에 숨겨져 있더라도 UX 리뷰를 필수로 수행해야 합니다. 추천된 UX 리뷰어를 기본값으로 설정하세요.

이는 다음 동작을 가지고 엔지니어링 프로젝트 페이지의 목록에서 리뷰어와 메인테이너를 선택합니다:

  • Slack 또는 GitLab 상태에 다음 문자열을 포함하는 사람을 선택하지 않습니다:
    • OOO, PTO, 육아휴가, 친구 및 가족, 또는 컨퍼런스.
    • GitLab 사용자의 바쁨 지표가 True로 설정된 경우.
    • 이모지가 다음 카테고리 중 하나인 경우:
      • 휴가 중 - 🌴 :palm_tree:, 🏖️ :beach:, ⛱ :beach_umbrella:, 🏖 :beach_with_umbrella:, 🌞 :sun_with_face:, 🎡 :ferris_wheel:, 🏙 :cityscape:
      • 병가 중 - 🌡️ :thermometer:, 🤒 :face_with_thermometer:
      • 가용한 리뷰량 - 🔴 :red_circle:
      • 집중 모드 - 💡 :bulb: (팀의 작업에 집중 중인 경우)
  • 이미 배정된 리뷰 수가 선택한 “리뷰 한계”와 동일하거나 그 이상인 사람을 선택하지 않습니다. 리뷰 한계는 한 번에 처리할 수 있는 리뷰의 최대 수입니다. 리뷰 한계는 Slack 또는 GitLab 상태를 사용하여 설정합니다:
    • 0️⃣ - :zero: ( :red_circle:와 유사함)
    • 1️⃣ - :one:
    • 2️⃣ - :two:
    • 3️⃣ - :three:
    • 4️⃣ - :four:
    • 5️⃣ - :five:

    보안 그룹의 기본 브랜치를 대상으로 하지 않는 병합 요청의 리뷰 요청은 계산되지 않습니다. 이러한 MR은 일반적으로 백포트이며, 메인테이너 또는 리뷰어가 일반적으로 그것을 검토하는 데 많은 시간이 필요하지 않습니다.

  • 리뷰에 대한 헝그리 지수: Slack 또는 GitLab 상태의 이모지가 🔵 :large_blue_circle:인 팀 멤버가 선택될 확률이 더 높습니다. 이는 리뷰어와 연수생 메인테이너 모두에게 적용됩니다.
    • 🔵 :large_blue_circle:를 가진 리뷰어는 다른 리뷰어보다 2배 더 선택될 확률이 높습니다.
    • 🔵 :large_blue_circle:를 가진 연수생 메인테이너는 다른 리뷰어보다 3배 더 선택될 확률이 높습니다.
  • GitLab 상태 이모지가 🔶 :large_orange_diamond: 또는 🔸 :small_orange_diamond:인 경우 선택될 확률이 반으로 줄어듭니다.
  • 동일한 브랜치 이름에 대해 항상 동일한 리뷰어와 메인테이너를 선택합니다 (1번 항목처럼 OOO 상태가 변경되지 않는 한). 이를 통해 백포트 브랜치에서 안정적으로 사용할 수 있도록 앞쪽의 ce-ee- 및 뒤쪽의 -ce-ee를 제거합니다.
  • GitLab 상태 이모지가 Ⓜ :m:인 경우에는 해당 프로젝트의 메인테이너로만 리뷰어로서 제안됩니다.

Roulette 대시보드에는 다음이 포함되어 있습니다:

  • 지난 7일 및 30일간의 배정 이벤트.
  • 각 사람별 현재 할당된 병합 요청.
  • 다른 기준별로 정렬.
  • 수동 리뷰어 룰렛.
  • 현지 시간 정보.

더 많은 정보는 룰렛 README를 확인하세요.

승인 지침

관리자의 책임 부분에 설명된 대로, 도메인 전문지식을 가진 유지 관리자에 의해 병합 요청이 승인되고 병합될 것을 권장합니다. 첫 번째 리뷰어의 선택적 승인은 여기에 포함되지 않습니다. 그러나 병합 요청은 개요 부분에 설명된 대로 유지자에게 전달되기 전에 리뷰어에 의해 검토되어야 합니다.

병합 요청에 포함된 내용 다음 중 하나에 의해 승인되어야 함
~backend 변경 사항 1 백엔드 유지자.
~database 마이그레이션 또는 비용이 많이 드는 쿼리 변경2 데이터베이스 유지자. 자세한 내용은 데이터베이스 리뷰 지침을 참조하십시오.
~workhorse 변경 Workhorse 유지자.
~frontend 변경 사항 1 프론트엔드 유지자.
~UX 사용자를 위한 변경 사항 3 제품 디자이너. 자세한 내용은 디자인 및 사용자 인터페이스 지침을 참조하십시오.
새로운 JavaScript 라이브러리 추가 1 - 프론트앤드 기초 구성원이라면 라이브러리가 번들 크기를 크게 늘리는 경우.
- GitLab에서 사용하기 위해 승인되지 않은 라이브러리를 사용한다면 법률 부서 구성원에게 요청하십시오.

라이선스 호환성에 대한 자세한 정보는 GitLab 라이선스 및 호환성 문서에서 확인할 수 있습니다.
GitLab에 새로운 종속성 또는 파일 시스템 변경 추가 - 배포 팀 구성원이라면 배포 팀과 함께 작업하는 방법을 자세히 살펴보십시오.
- RubyGems의 경우 AppSec 리뷰를 요청하십시오.
~documentation 또는 ~UI 텍스트 변경 적절한 DevOps 단계 그룹의 과제를 기반으로 기술 작성자.
개발 지침 변경 리뷰 프로세스를 따르고 해당 승인을 얻으십시오.
종단 간 비종단 간 변경 4 테스트 소프트웨어 엔지니어.
종단 간 변경만 4 또는 MR 작성자가 테스트 소프트웨어 엔지니어인 경우 품질 유지자.
새로운 또는 업데이트된 응용 프로그램 한도 제품 관리자.
분석 계측 (텔레메트리 또는 분석) 변경 분석 계측 엔지니어.
기능 스펙 추가 또는 변경 품질 유지자 또는 품질 리뷰자.
GitLab에 새로운 서비스 추가 (Puma, Sidekiq, Gitaly 예시) 제품 관리자. 자세한 내용은 GitLab에 서비스 구성 요소 추가를 참조하십시오.
인증 관련 변경 관리:인증 부서. 자세한 내용은 그룹 페이지의 코드 검토 섹션(code review section on the group page)을 확인하십시오. 팀에서 리뷰가 필요한 파일의 패턴은 CODEOWNERS 파일의 Authentication 섹션에 나열되어 있으며, 이 팀은 이러한 파일을 수정하는 모든 병합 요청의 승인자 목록에 나열될 것입니다.
사용자 정의 역할 또는 정책 관련 변경 관리:인가 엔지니어.
  1. JavaScript 스펙 이외의 스펙은 ~backend 코드로 간주됩니다. Haml 마크업은 ~frontend 코드로 간주됩니다. 그러나 Haml 템플릿의 루비 코드는 ~backend 코드로 간주됩니다. 의심이 들 때, 프론트엔드 및 백엔드 리뷰를 요청하십시오.
  2. 병합 요청이 비용이 많이 드는 쿼리를 도입할 위험이 있는 경우 데이터베이스 유지자의 지침을 받는 것을 장려합니다. 그들이 자문을 제공할 수 있도록 의문의 코드 라인에 SQL 쿼리에 대한 의견을 달라는 것이 가장 효율적입니다.
  3. 사용자를 대상으로 한 변경 사항에는 시각적 변경 사항(얼마나 작은지와 무관하게)과 화면 판독기가 내용을 발표하는 방식에 영향을 주는 렌더링된 DOM에 대한 변경 사항이 포함됩니다. 전문적인 제품 디자이너를 보유하지 않는 그룹은 변경 사항을 승인하려면 커뮤니티 기여인 경우를 제외하고 제품 디자이너의 승인이 필요하지 않습니다.
  4. 종단 간 변경 사항에는 qa 디렉토리의 모든 파일이 포함됩니다.

CODEOWNERS 승인

일부 병합 요청은 특정 그룹의 필수 승인이 필요합니다. 정의에 대해서는 .gitlab/CODEOWNERS를 참조하십시오.

.gitlab/CODEOWNERS의 필수 섹션은 다음과 같은 경우에만 필수적으로 사용해야 합니다.

  • 규정 준수
  • 가용성
  • 보안

필수 섹션을 추가할 때는 병합 요청 속도에 미치는 영향을 추적해야 합니다. 좋은 예시는 Verify issue를 참조하세요.

다른 모든 경우에는 필수 섹션을 사용해서는 안 됩니다. 우리는 엄격함보다 책임을 선호합니다.

또한, 현재 단일체(monolith)의 구조는 병합 요청이 겉으로 보기에는 관련 없는 부분에 영향을 미칠 가능성이 높습니다. 여러 필수 승인은 이러한 병합 요청에 저자가 승인을 요청해야 하는 것을 의미하는데, 이는 효율적이지 않습니다.

이를 개선하기 위한 노력은 다음에서 이루어지고 있습니다.

승인 체크리스트

이 체크리스트는 병합 요청(MR)의 작성자, 리뷰어 및 유지보수자가 변경 내용을 품질, 성능, 신뢰성, 보안, 관측성 및 유지보수에 대한 높은 영향을 분석했는지 확인하기 위한 것입니다.

체크리스트 사용은 소프트웨어 엔지니어링의 품질을 향상시킵니다. 이 체크리스트는 GitLab 코드베이스의 기여자들의 기술을 지원하고 강화시키는 간단한 도구입니다.

품질

추가적인 품질 안내를 위해 테스트 엔지니어링 프로세스를 참조하세요.

  1. 본 MR을 코드 리뷰 지침에 따라 자체 검토했습니다.
  2. 이 변경 사항이 영향을 미친 코드에 대해 자동화된 테스트(테스팅 가이드)가 사용자에게 높은 중요성을 갖는 기능을 검증한다고 믿습니다(여기에는 모든 테스트 수준을 고려합니다).
  3. 기존의 자동화된 테스트가 위의 기능을 포함하지 않는 경우, 해당 기능을 추가하거나 해당 MR에 링크된 자동화 테스트 갭을 설명하는 이슈를 추가했습니다.
  4. 이 변경 사항이 GitLab.com 호스팅 고객 및 자체 관리 고객에 미치는 기술적 영향을 고려했습니다.
  5. 이 변경 사항이 시스템의 프론트엔드, 백엔드 및 데이터베이스 부분에 미치는 영향을 적절히 고려하고, ~ux, ~frontend, ~backend, 및 ~database 레이블을 적용했을 것입니다.
  6. 본 MR을 지원되는 모든 브라우저에서 테스트했거나, 해당 테스트가 필요하지 않음을 확인했습니다.
  7. 이 변경이 업데이트 간의 하위 호환성이 있는지 확인했거나, 이에 해당하지 않는지 결정했습니다.
  8. EE(Enterprise Edition) 콘텐츠를 적절히 분리했거나 본 MR이 FOSS(Free and Open Source Software) 전용인지 확인했습니다.
  9. 본 MR이 EE 및 FOSS에 서로 다른 방식으로 영향을 줄 수 있는 경우, FOSS 컨텍스트에서 CI 파이프라인을 실행하는 것을 고려했습니다.
  10. 기존 데이터가 놀랍도록 다양할 수 있다는 것을 고려했습니다. 예를 들어, 새로운 모델 유효성 검사가 기존 레코드를 깨뜨릴 수 있습니다. 기존 데이터가 유효성 검사를 통과할 것이라는 확인을 하지 않은 경우, 존재하는 데이터에 대한 검증을 필수적으로 처리하는 대신 선택 사항으로 만드는 것을 고려했습니다.
  11. 테스트가 경고와 함께 통과하며, 실패한 작업에 ‘Flaky test ‘<경로/테스트>’이(가) 이 MR에서 변경된 파일 목록에 포함되어 있다.’라는 텍스트가 포함된 경우, 이 테스트를 수정하거나 해당 테스트가 무시될 수 있는 이유를 설명했습니다.
성능, 신뢰성, 가용성
  1. 이 MR이 성능에 해를 끼치지 않았다고 확신하거나, 리뷰어가 성능 영향을 평가하는 데 도움을 청했습니다. (병합 요청 성능 지침)
  2. MR 설명에 데이터베이스 리뷰어를 위한 정보를 추가했거나, 이러한 정보가 불필요하다고 결정했습니다.
  3. 이 변경의 가용성 및 신뢰성 위험을 고려했습니다.
  4. 미래 예측된 성장을 기반으로 한 확장성 위험을 고려했습니다.
  5. 이 변경이 평균 고객보다 비교적 많은 데이터를 갖는 대규모 고객에 미치는 성능, 신뢰성 및 가용성 영향을 고려했습니다.
  6. 이 변경이 GitLab을 최소 시스템에서 실행하는 고객에 미치는 성능, 신뢰성 및 가용성 영향을 고려했습니다.
관측성 인스트루먼테이션
  1. 관측성을 통해 디버깅 및 적극적인 성능 향상을 위한 충분한 인스트루먼테이션을 포함했습니까? 예시에서 피처 플래그, 로깅 및 인스트루먼테이션 추가를 참조하세요.
문서
  1. 변경 로그 트레일러스를 포함했거나 필요하지 않다고 판단했습니다.
  2. 문서를 추가/업데이트했거나 이 MR에 대한 문서 변경이 필요하지 않다고 결정했습니다.
보안
  1. 이 MR에 자격 증명이나 토큰, 권한 부여 및 인증 방법, 또는 보안 검토 지침에서 설명된 기타 항목에 대한 변경이 포함되어 있는지 확인했으며, ~security 라벨을 추가하고 @gitlab-com/gl-security/appsec를 언급했습니다.
  2. 내부 애플리케이션 보안 검토에 대한 문서를 검토했고, 이 변경에 대해 보안 검토가 필요한 경우에 요청했습니다.
  3. 이 MR을 차단하는 보안 스캔 결과가 있는 경우(병합 요청 승인 정책에 의해):
    • 실제 문제가 되는 결과인 경우, 병합 요청이 병합되기 전에 수정되어야 합니다. 이로써 병합 요청 승인 정책에 따른 AppSec 승인이 제거됩니다.
    • 잘못된 양성 결과가 있는 경우, 위험 수용을 논의해야 하거나 의문스러운 사항이 있는 경우 @gitlab-com/gl-security/appsec를 핑했습니다.
배포
  1. 이 변경의 위험이 높을 수 있으므로 이 변경에 대해 기능 플래그를 사용하는 것을 고려했습니다.
  2. 기능 플래그를 사용하는 경우, 생산 환경에서 테스트하기 전에 스테이징에서 변경 사항을 테스트할 계획을 세우고, 모든 고객에게 배포하기 전에 일부 고객에게 먼저 배포할 것을 고려했습니다.
  3. 완료 정의에 따라 인프라 부서에 기본 설정 또는 새로운 설정 변경을 알렸거나, 이것이 불필요하다고 판단했습니다.
준수
  1. 올바른 MR 유형 라벨이 적용되었는지 확인했습니다.

병합 요청 작성자의 책임

가장 좋은 해결책을 찾고 이를 구현하는 책임은 병합 요청 작성자에게 있습니다. 작성자 또는 직접 책임을 지는 개인(DRI)은 코드 리뷰 수명 주기 전체에 걸쳐 병합 요청의 담당자로 지정됩니다. 만약 직접 책임을 지는 개인으로 자신을 지정할 수 없는 경우, 리뷰어에게 이를 요청하세요.

유지보수자가 승인하고 병합하기 위해 리뷰를 요청하기 전에, 다음 사항에 대해 확신해야 합니다.

  • 이 변경 사항이 의도한 문제를 실제로 해결하는가.
  • 가장 적합한 방식으로 그것을 수행하는가.
  • 모든 요구 사항을 충족하는가.
  • 남은 버그, 논리적 문제, 아직 해결되지 않은 모서리 경우나 알려진 취약점이 있는가.

리뷰어와의 불필요한 주고받음을 피하고 싶다면, 자신의 병합 요청에 대해 자체 리뷰를 수행하고, 코드 리뷰 지침을 따르세요. 이 자체 리뷰 중에, 결정이나 교환을 한 코드 행 또는 상황에 대한 설명이 리뷰어가 코드를 더 쉽게 이해하는 데 도움이 되는 곳에 주석을 달아 보세요.

해당해결책에 대한 요구 수준에 도달하기 위해, 작성자는 적절한 과정에서 타인을 포함시키기를 기대합니다.

이들은 서로 다른 해결책을 논의하거나 구현을 검토하기 위해 도메인 전문가에게 연락하기를 권장받습니다, 제품 관리자와 UX 디자이너에게 혼란을 해소하거나 최종 결과가 예상과 일치하는지 확인하기 위해, 데이터베이스 전문가에게 데이터 모델이나 특정 쿼리에 대한 의견을 구하기 위해 또는 기타 개발자에게 해결책에 대한 심층적 검토를 받기 위해.

특정한 기능을 제공하려면 많은 병합 요청이 필요할 것으로 예상된다면(예: POC(Proof of Concept)를 만들었고 해당 기능이 10개 이상의 병합 요청으로 이루어질 것으로 분명한 경우), 해당 기능의 필요한 이해를 갖춘 리뷰어 및 유지보수자를 식별하고 병합 요청을 이러한 리뷰어에게 직접 지시하세요. 이러한 리뷰어를 찾는 데 가장 적합한 DRI는 EM 또는 스태프 엔지니어입니다. 같은 문맥을 가진 병합 요청의 안정적인 리뷰어 상대방을 갖는 것은 효율성을 향상시킵니다.

병합 요청이 하나 이상의 도메인을 다룬다면(예: Dynamic Analysis 및 GraphQL), 각 도메인 전문가에게 검토를 요청하세요.

작성자가 병합 요청이 도메인 전문가의 의견이 필요한지 확신할 수 없는 경우, 그것은 필요하다는 것을 의미합니다. 그러지 않으면, 그들이 자신의 해결책에 대해 필요한 수준의 확신을 가지고 있지 않을 가능성이 높습니다.

리뷰를 요청하기 전에, 작성자가 리뷰어에게 병합 요청의 차이점에 대해 중요한 사항 또는 추가로 설명이 필요한 사항을 알리기 위해 주석을 제출하기를 요청합니다. 주석이 필요한 내용 예시로는 다음이 있습니다.

  • 린팅 규칙 추가(RuboCop, JS 등).
  • 라이브러리 추가(Ruby gem, JS lib 등).
  • 명백하지 않은 곳에 부모 클래스 또는 메소드에 대한 링크.
  • 변경 사항을 보충하기 위해 수행된 벤치마킹.
  • 잠재적으로 보안에 취약한 코드.

리뷰어가 솔루션을 확인하기 위해 프로젝트, 스니펫 또는 기타 자산이 필요한 경우, 리뷰어가 검증을 할 수 있도록 해당 자산에 대한 액세스 권한이 있는지 확인하세요.

리뷰어를 지정할 때, 다음이 도움이 될 수 있습니다.

  • MR에 각 리뷰어가 제공해야 하는 유형의 리뷰를 나타내는 주석을 추가합니다.
    • 예를 들어, MR이 데이터베이스 쿼리를 변경하고 백엔드 코드를 업데이트하는 경우, MR 작성자는 먼저 ~backend 리뷰와 ~database 리뷰가 필요합니다. 리뷰어에게 할당함과 동시에 각 리뷰어에게 어떤 도메인을 리뷰해야 하는지 알리는 주석을 MR에 추가합니다.
    • 많은 GitLab 팀 구성원은 두 개 이상의 영역의 도메인 전문가이기 때문에, 때로는 이러한 유형의 주석이 없으면 어느 유형의 리뷰를 제공해야 하는지 모호할 수 있습니다.
    • MR 리뷰 유형에 대한 명확성은 MR 작성자에게 효율적입니다. 왜냐하면 그들은 원하는 리뷰 유형을 받게되고, 리뷰어에게는 즉시 어떤 유형의 리뷰를 제공해야 하는지가 분명하기 때문입니다.
    • 예시 1
    • 예시 2

다음을 피하세요.

  • 리뷰어가 필요로 하는 경우에만 소스 코드에 TODO 주석(위에 언급한)을 추가하지 않습니다. TODO 주석이 실제 작업으로 인해 추가된 경우, 관련 이슈에 대한 링크를 포함하세요.
  • 코드가 하는 일을 설명하는 주석만 추가하지 않습니다. 비-TODO 주석을 추가하려면 왜을 설명해야 합니다.
  • 실패한 테스트가 있는 병합 요청의 유지보수자 검토를 요청하지 않습니다. 테스트가 실패하고 리뷰를 요청해야 하는 경우 설명이 담긴 주석을 꼭 남기세요.
  • 이메일이나 Slack을 통해 유지보수자에게 과도하게 언급하지 않습니다(유지보수자가 Slack을 통해 접근 가능한 경우). 병합 요청에 리뷰어를 추가할 수 없는 경우, 해당 병합 요청에 대해 주석으로 유지보수자를 언급하는 것은 허용되며, 다른 모든 경우에 리뷰어를 추가하는 것으로 충분합니다.

이것은 리뷰어의 시간을 절약하고 작성자가 일찍 실수를 발견하는 데 도움이 됩니다.

리뷰어의 책임

리뷰어는 선택한 솔루션의 구체적인 내용을 검토하는 책임이 있습니다.

병합 요청을 검토하세요 신중히.

병합 요청이 기여 수락 기준을 모두 충족하는지 확인하세요.

일부 병합 요청은 특정 분야 전문가의 도움이 필요할 수 있습니다. 리뷰어는 해당 분야의 전문가가 아닌 경우 다음 중 하나를 할 수 있습니다.

  • 병합 요청을 검토하고 다른 리뷰어나 유지 보수자로부터 다시 리뷰를 요청합니다. 해당 전문가는 다른 리뷰어나 유지 보수자가 될 수 있습니다.
  • 더 적합한 다른 리뷰어에게 리뷰를 넘깁니다.
  • 분야 전문가를 사용할 수 없는 경우, 최선을 다해 검토합니다.

병합 요청을 다음 중 하나에 해당하는 경우 작은 병합 요청으로 분할할 것을 저자에게 안내해야 합니다:

  • 너무 큰 경우.
  • 하나 이상의 문제를 수정하는 경우.
  • 하나 이상의 기능을 구현하는 경우.
  • 추가 리스크를 초래하는 높은 복잡성을 갖는 경우.

저자는 현재 유지자와 리뷰어들이 분할된 MR을 검토하도록 요청하거나 새로운 그룹의 유지자와 리뷰어로부터 검토를 요청할 수 있습니다.

요구 사항을 모두 충족하는지 확신이 든다면 다음을 수행해야 합니다:

  • 승인을 선택합니다.
  • @를 사용하여 작성자를 언급하여 ‘병합 요청이 검토되고 승인되었음을 알리는 할 일 알림’을 생성하고, 그들에게 알려줍니다.
  • 유지자로부터 검토를 요청합니다. 도메인 전문성을 갖춘 유지자에게 기본으로 요청하지만, 해당 유지자가 없거나 병합 요청이 도메인 전문가에게 검토 받아야 할 필요가 없다고 생각되는 경우 리뷰어 룰렛 제안을 따르는 것도 괜찮습니다.
  • 리뷰어로부터 자신을 삭제합니다.

유지자의 책임

유지자는 GitLab 코드베이스의 전반적인 건강, 품질, 일관성에 책임을 지고 있습니다. 이에 따라 그들의 검토는 주로 전체적인 아키텍처, 코드의 구조, 관심사의 분리, 테스트, DRYness, 일관성 및 가독성과 같은 사항에 중점을 둡니다.

따라서 유지자의 작업은 특정 도메인에 대한 지식과 관계없이 모든 팀과 모든 제품 영역에서의 병합 요청을 검토, 승인 및 병합할 수 있습니다.

유지자는 병합 요청의 수락 기준이 합리적으로 충족되었는지를 확인하는 DRI(책임 소유자)입니다. 일반적으로 품질은 모두의 책임이지만, MR의 유지자들은 MR이 해당 일반 품질 기준을 충족하는지 확인할 책임이 있습니다.

이는 후속 이슈에서 기술 부채 생성을 피하기 위한 것입니다.

유지자가 병합 요청이 상당히 중요하거나 도메인 전문가의 검토가 필요하다고 판단하면, 유지자들은 다음과 같은 경우에 다른 리뷰어 또는 유지자로부터 검토를 요청할 수 있습니다. 여기에 유지자가 검토 중에 이용한 예가 있습니다:

유지자들은 병합하기 전에 선택한 솔루션의 구체적인 내용도 심사하기 위해 최선을 다하지만, 그들이 도메인 전문가가 아닐 수 있으므로 불합리한 시간 투자 없이는 그렇게 할 수 없을 수도 있습니다. 이 경우에는 작성자와 이전 리뷰어들의 심사에 의지하여, 본인의 주요 책임에 중점을 둔다는 이유로 결정을 지연시킬 수 있습니다.

개발자이자 유지자인 경우, 그들이 리뷰어로 참여했던 병합 요청은 최종적으로 승인하고 병합하는 유지자로 선택되지 않는 것이 좋습니다.

유지자는 병합하기 전에 필요한 승인자에 의해 병합 요청이 승인되었는지 확인해야 합니다. 아직 다른 사람들로부터 추가 승인을 기다리는 경우 자신을 리뷰어에서 삭제한 후 @를 사용하여 작성자를 언급하고 이유를 설명하는 댓글을 남겨야 합니다. 코드를 병합하는 경우 리뷰어로 남아야 합니다.

특정 병합 요청은 안정적인 브랜치를 대상으로 할 수 있습니다. 해당 요청을 처리하는 방법에 대한 개요는 패치 릴리스 런북을 참조하세요.

병합 후, 유지자는 병합 요청에 나열된 리뷰어로 남아 있어야 합니다.

리뷰어 기능 도그푸딩

2021년 3월 18일, 리뷰어 기능을 효율적으로 사용하여 일관성 있게 도그푸딩하는 데에 목적을 둔 프로세스가 업데이트되었습니다.

위 기능에 반영된 변경 내용에 대한 요약은 다음과 같습니다.

  • 병합 요청 저자와 DRI(책임 소유자)는 담당자로 남습니다.
  • 작성자는 리뷰어로 지정된 사용자를 할당함으로써 리뷰를 요청합니다.
  • 리뷰어는 리뷰와 승인이 완료되면 자신을 언제든지 할당 해제합니다.
  • MR을 병합하는 마지막 승인자는 여전히 리뷰어로서 할당됩니다.

Best practices

모두

  • 친절하게 대해주세요.
  • 프로그래밍 결정의 많은 부분은 주관적이라는 것을 받아들이세요. 어떤 것을 선호하는지, 어떤 점과 어떤 것을 희생으로 여기는지 논의하고 빨리 결론을 내리세요.
  • 질문하세요. 명령하지 마세요. (“이 :user_id라는 이름이 어떤 것이라고 생각하세요?”)
  • 설명을 요청하세요. (“이해가 안 가는데요. 설명해주시겠어요?”)
  • 코드의 선택적 소유를 피하세요. (“내 것”, “내 것이 아님”, “네 것”)
  • 개인적인 특성을 가리키는 용어를 사용하는 것을 피하세요. (“어리석은”, “바보같은”). 모두가 지적이며 선량하다고 가정하세요.
  • 명확하게 표현하세요. 온라인에서 다른 사람들이 항상 여러분의 의도를 이해하지 못할 수 있습니다.
  • 겸손하게 행동하세요. (“잘 모르겠어요 - 찾아보겠습니다.”)
  • 과장하지 마세요. (“항상”, “절대로”, “끝없이”, “없음”)
  • 풍자의 사용에 주의하세요. 우리가 하는 모든 것은 공개적입니다. 오랜 동료에게 웃음 가득한 장난으로 보일 수 있는 것이 새로운 프로젝트에 참여한 사람에게는 무례하고 환영받지 않는 것으로 느껴질 수 있습니다.
  • “이해 못 했다” 또는 “대안적인 해결책:”이라는 의견이 너무 많이 있을 경우 일대일 채팅이나 영상 통화를 고려해보세요. 일대일 토론 내용을 요약하는 후속 의견을 게시하세요.
  • 특정 사람에게 질문할 경우 댓글을 항상 그 사람을 언급하며 시작하세요. 그렇게 함으로써 그 사람이 “언급됨”으로 설정되어 있을 경우 화면에 보이게 하고, 다른 사람들은 응답할 필요가 없음을 이해할 수 있도록 합니다.

병합 요청이 리뷰되는 과정

코드 리뷰는 여러 번의 반복을 거쳐 진행될 수 있는 프로세스이며 리뷰어는 이전에 볼 수 없었던 것을 나중에 발견할 수 있습니다.

  • 코드의 첫 번째 리뷰어는 당신 입니다. 새로운 브랜치를 푸시하기 전에 전체 변경 내용을 읽어보세요. 이해가 되나요? 전체 변경 사항의 전반적인 목적과 무관한 내용은 포함되지 않았는지 확인하세요. 디버깅 코드를 삭제하는 것을 잊지는 않았나요?
  • 병합 요청 가이드라인에 기술된 대로 자세한 설명을 작성하세요. 제품 기능이나 코드베이스 영역에 익숙하지 않은 리뷰어도 있는데, 철저한 설명은 여러 리뷰어가 요청을 이해하고 효과적으로 테스트할 수 있도록 도와줍니다.
  • 만약 여러분의 변경 사항이 다른 변경 사항에 의존한다면 설명란에 그것을 표기하고 병합 요청 의존성을 설정하세요.
  • 리뷰어의 제안에 감사드립니다. (“좋은 제안이네요. 그 부분을 수정하겠습니다.”)
  • 개인적으로 받아들이지 마세요. 리뷰는 코드에 대한 것입니다, 당신에 대한 것이 아닙니다.
  • 코드가 존재하는 이유를 설명하세요. (“그것은 이유 때문에 그렇게 되었습니다. 만약 이 클래스/파일/메서드/변수의 이름을 바꾸면 더 명확해질까요?”)
  • 상관 없는 변경 사항과 리팩터링은 나중에 병합 요청/이슈로 분리하세요.
  • 리뷰어의 관점을 이해하려고 노력하세요.
  • 모든 의견에 답변하려고 노력하세요.
  • 병합 요청 작성자는 완전히 해결한 쓰레드만 해결합니다. 만약 여전히 모든 의견에 열린 답변, 제안, 질문 또는 다른 것이 있다면 리뷰어가 해결하도록 남겨두어야 합니다.
  • 모든 피드백이 MR에 병합되기 전에 반드시 추천된 변경 사항을 반영해야한다고 가정해서는 안 됩니다. 이것은 MR 작성자와 리뷰어가 판단하는 사안이며, 해당 사항이 필요한지 아니면 주어진 MR이 병합된 후 미래의 피드백을 처리할 수 있는 후속 이슈를 만들어야 하는지를 결정해야 합니다.
  • 이전 피드백을 반영한 커밋은 별도의 커밋으로 브랜치에 추가하세요. 브랜치가 병합할 준비가 될 때까지 squash 하지 마세요. 리뷰어는 이전 피드백을 기반으로 개별 업데이트를 읽을 수 있어야 합니다.
  • 다시 리뷰가 필요할 때 리뷰어에게 새로운 리뷰를 요청하세요. 리뷰를 요청할 수 있는 권한이 없다면 리뷰어를 @로 언급하세요.

리뷰를 요청하는 경우

병합 요청을 검토해야 할 준비가 되었다면, 초기 리뷰를 요청하여 승인 지침을 기반으로 리뷰어를 선택하세요.

병합 요청이 여러 영역에 대해 검토되어야 하는 경우, 어떤 영역을 리뷰어가 리뷰해야 하는지 그리고 어느 단계에서(첫 번째 또는 두 번째) 명시하는 것이 좋습니다. 여러 영역에 대한 리뷰어 자격이 있는 팀 멤버를 위해서, 예를 들어, 백엔드프론트엔드 관심이 모두 있는 경우에는 리뷰어를 다음과 같이 언급할 수 있습니다: @john_doe님 ~백엔드를 리뷰해주시겠어요? 또는 @jane_doe - 이 MR을 ~프론트엔드 관리자 리뷰해주실래요?

또한 workflow::ready for review 레이블을 사용할 수 있습니다. 이는 병합 요청이 검토할 준비가 되었음을 의미합니다. 이 레이블은 시간 압박이 없는 경우에만 사용하는 것이 좋으며, 병합 요청이 리뷰어에게 할당되었는지 확인하세요.

첫 번째 리뷰어로부터 승인을 받은 경우, 유지자에게 전달할 수 있습니다. 도메인 전문가를 선택하는 것이 좋으며, 그 외의 경우에는 리뷰어 룰렛 권장 사항에 따르거나 ready for merge 레이블을 사용하세요.

가끔씩 유지자가 리뷰를 할 수 없을 수 있습니다. 그들이 사무실을 비출 수도 있고 용량을 다한 상황일 수도 있습니다. 당신은 그 유지자의 가용성을 확인할 수 있고 확인해야 합니다. 룰렛에 의해 추천된 유지자가 사용할 수 없다면, 그 목록에서 다른 사람을 선택해야 합니다.

병합 요청의 리뷰는 작성자의 책임입니다. 만약 “리뷰를 위해 준비 완료” 상태로 너무 오랫동안 유지된다면, 특정한 리뷰어로부터 리뷰 요청을 하는 것이 좋습니다.

리뷰 지원

여유가 있는 GitLab 엔지니어는 정기적으로 검토할 병합 요청 목록을 확인하고 원하는 병합 요청에 리뷰어로 자신을 추가할 수 있습니다.

병합 요청 검토

변경의 필요성을 이해합니다(버그 수정, 사용자 경험 향상, 기존 코드 리팩터링). 그런 다음:

  • 반복 횟수를 줄이기 위해 리뷰를 철저히 합니다.
  • 강하게 느끼는 아이디어와 그렇지 않은 아이디어를 소통합니다.
  • 문제를 해결하면서 코드를 간소화할 방법을 찾습니다.
  • 대체 구현을 제안하지만 작성자가 이미 고려했음을 가정합니다. (“여기에 사용자 정의 유효성 검사기를 사용하는 것은 어떻습니까?”)
  • 작성자의 관점을 이해하려고 노력합니다.
  • 브랜치를 확인하고 변경 사항을 로컬에서 테스트합니다. 수동 테스트를 얼마나 수행할지 결정할 수 있습니다. 테스트 결과 자동 테스트를 추가할 수 있는 기회가 발생할 수 있습니다.
  • 코드 일부를 이해하지 못할 경우, _그렇다고 말_합니다. 다른 사람들도 혼란스러워할 가능성이 높습니다.
  • 작성자가 제안을 처리/해결하기 위해 필요한 내용이 명확한지 확인합니다.
    • 의도를 전달하기 위해 관례적인 코멘트 형식을 사용하는 것을 고려하세요.
    • 비의무적인 제안의 경우, (비차단)으로 꾸며 작성자가 병합 요청 내에서 선택적으로 해결하거나 나중에 따라갈 수 있는지 알 수 있게 합니다.
    • Chrome/Firefox 추가 기능을 사용하여 관례적 코멘트 접두사를 적용할 수 있습니다.
  • 열린 종속성이 없는지 확인합니다. 차단 요소에 대해 확인합니다. 필요한 경우 작성자와 명확하게 해명하세요. 하나 이상의 오픈 MR이 차단되는 경우, MR 종속성을 설정합니다.
  • 코드 노트를 한 차례 게시한 후, “보기에 좋아 보입니다” 또는 “다룰 사항이 조금 있습니다”와 같은 요약 노트를 게시하는 것이 도움이 될 수 있습니다.
  • 리뷰 후 변경 사항이 필요한 경우 작성자에게 알려주세요.

경고: 병합 요청이 포크된 경우, 커뮤니티 기여에 대한 추가 지침을 확인해주세요.

병합 수행

결정을 내리기 전에:

  • 마일스톤을 설정하세요.
  • 올바른 MR 유형 레이블이 적용되었는지 확인하세요.
  • danger bot, 코드 품질 및 기타 보고서의 경고 및 오류를 고려하세요. 위반 사례에 강력한 근거가 제시되지 않는 한, 이러한 사항은 병합 전에 해결해야 합니다. 병합 시 작업에 실패한 경우에는 코멘트를 게시해야 합니다.
  • MR에 품질과 비품질 관련 변경 사항이 모두 포함된 경우, 품질 관련 변경 사항이 소프트웨어 엔지니어의 승인을 받은 후 사용자에게 노출되는 변경(백엔드, 프론트엔드 또는 데이터베이스)에 해당하는 관리자에 의해 병합되어야 합니다.

적어도 한 명의 관리자가 MR을 승인해야 병합됩니다. MR 작성자와 MR에 커밋을 추가한 사람은 MR을 승인할 권한이 없으며 MR을 승인해야 하는 관리자를 찾아야 합니다. 일반적으로 최종 필요한 승인자가 MR을 병합해야 합니다.

최종 승인자가 MR을 병합하지 않을 수 있는 시나리오:

  • 승인자가 승인 후 자동 병합 설정을 하지 않은 경우.
  • 승인자가 자신이 최종 승인자임을 깨닫지 못한 경우.
  • 승인자가 자동 병합 설정했지만 GitLab에서 해제된 경우.

위의 시나리오 중 하나라도 발생하면, MR 작성자는 모든 필요한 승인을 받았고 리포지토리에 병합 권한이 있으면 자체 MR을 병합할 수 있습니다. 이는 GitLab의 행동 편향 가치와 일치합니다.

본 정책은 GitLab의 변경 관리 컨트롤의 CHG-04 제어를 만족시키기 위해 시행됩니다.

gitlab-org/gitlab에서 이 정책을 시행하기 위해 다음과 같은 설정을 활성화했습니다:

gitlab-org/gitlabCODEOWNERS 파일에서 코드 소유자를 업데이트하려면 코드 소유자 승인 핸드북 섹션에 설명된 프로세스를 따르세요.

로컬에서 리베이스하거나 제안을 적용하는 등 일부 작업은 커밋을 추가하는 것과 동일하게 간주됩니다.
승인은 UI를 통해 리베이스하거나 /rebase 퀵 액션으로 진행할 때 제거되지 않습니다.

병합할 준비가 되었을 때:

경고: 병합 요청이 포크된 경우, 커뮤니티 기여에 대한 추가 지침을 확인해주세요.

  • 병합 요청에 많은 커밋이 있는 경우 병합 및 축소 기능을 사용하는 것을 고려하세요. 코드를 병합할 때, 작성자가 이미 이 옵션을 설정한 경우에만 관리자가 squash 기능을 사용해야 하며, 또는 병합 요청이 명확히 더러운 커밋 이력을 포함하고 있는 경우 작성자와 다시 이야기하는 대신 커밋을 축소하는 것이 더 효율적일 수 있습니다. 그렇지 않은 경우, 몇 개의 커밋만 있는 경우 작성자의 설정을 존중하여 이를 축소하지 않을 것입니다.
  • 병합 요청의 파이프라인 탭으로 가서 파이프라인 실행을 선택하세요. 그런 다음 개요 탭에서 자동 병합을 활성화하세요. 다음 사항을 유의하세요:
    • 기본 브랜치가 손상되었을 경우, 병합 요청을 병합하지 말아 주세요만약 매우 구체적인 경우에 해당하지 않는 다른 경우, 이 핸드북 지침을 따르세요.
    • 최신 파이프라인이 승인되기 전에 만들어진 경우, 완전한 RSpec suite가 실행되었는지 확인하기 위해 새로운 파이프라인을 시작하세요. 백엔드 변경 사항이 포함되지 않은 경우에만 이 단계를 건너뛸 수 있습니다.
    • 최신 병합된 결과 파이프라인4시간 이내에 만들어진 경우, 병합 요청이 대상 브랜치에 충분히 가까울 경우 새로운 파이프라인을 시작하지 않고 병합할 수 있습니다.
  • MR을 자동으로 병합하면, 그 이후에 발견되는 사항들을 처리하기 위해 책임져야 합니다.
  • 병합 및 축소를 설정한 병합 요청의 경우, 축소된 커밋의 기본 커밋 메시지는 병합 요청 제목에서 가져옵니다. 병합하기 전에 더 정보가 있는 커밋 메시지를 선택할 것을 권장합니다.

병합된 결과 파이프라인 덕분에 작성자는 이제 브랜치를 자주 리베이스할 필요가 없어졌습니다(충돌이 있는 경우를 제외하고). 이는 MR 결과 파이프라인이 이미 main의 최신 변경 사항을 포합하기 때문에 리베이스의 최종 결과를 요청해야 한다는 절차를 줄이고 있으며, 관리자는 MR 파이프라인을 시작하고 자동 병합만 설정하면 될 뿐입니다. 이 단계는 MR결과 파이프라인이 생성된 시간을 기준으로 main의 최신 상태를 테스트합니다.

커뮤니티 기여

경고: 악의적인 코드를 방지하기 위해 모든 변경 사항을 _merged results pipeline_을 시작하기 전에 주의 깊게 검토하십시오.

넓은 커뮤니티 기여자들에 의해 추가된 병합 요청을 검토할 때:

  • 루비 젬 및 노드 패키지와 같은 새로운 종속성 및 종속성 업데이트에 특별히 주의하십시오. Gemfile.lock이나 yarn.lock과 같은 파일의 변경 사항이 사소해 보일 수 있지만 악성 패키지를 가져올 수 있습니다.
  • 특히 문서 MRs의 링크와 이미지를 신중하게 검토하십시오.
  • 의심스러울 때에는 수동으로 병합 요청 파이프라인을 시작하기 전에 @gitlab-com/gl-security/appsec에서 누군가에게 리뷰하도록 요청하십시오.
  • 병합 요청이 현재 마일스톤에 포함될 가능성이 높을 때에만 마일스톤을 설정하십시오. 이는 병합될 때 혼란을 피하고 아직 준비되지 않았을 때 마일스톤을 너무 자주 이동하는 것을 피하기 위함입니다.

복제 요청의 원본 브랜치가 타깃 브랜치보다 1,000개 이상의 커밋을 뒤처지면:

  • 저자에게 다시베이스하도록 요청하거나, MR에 “대상 브랜치로 병합할 수 있는 멤버의 커밋 허용”이 활성화되어 있다면 스스로 리베이스를 고려하십시오.
  • 최근 변경 사항의 맥락에서 MR을 검토하면 숨겨진 런타임 충돌을 방지하고 일관성을 유지하는 데 도움이 될 수 있습니다. 변경의 성격에 따라, MR이 1,000개 미만의 커밋을 뒤처지고 있다면 리베이스하는 것도 고려해야 합니다.
  • 강제 푸시는 기여자를 당황시킬 수 있으므로 리베이스를 수행했음을 알리는 것이 좋으며, 기여자가 MR에서 활발하게 작업 중일 때는 먼저 확인하는 것이 좋습니다.
  • 리베이스는 대부분 /rebase 빠른 조치로 GitLab에서 수행할 수 있습니다.

커뮤니티 병합 요청 이관

MR이 추가 변경이 필요하지만 저자가 장기간 응답하지 않거나 MR을 완료할 수 없을 때, GitLab은 이슈 및 병합 요청에 대한 종료 정책에 따라 이를 이관할 수 있습니다. GitLab 엔지니어(일반적으로 병합 요청 코치)는:

  1. MR에 코멘트를 추가하여 병합되어야 함을 알립니다.
  2. MR에 라벨 ~"coach will finish"를 추가합니다.
  3. 메인 브랜치에서 새로운 기능 브랜치를 생성합니다.
  4. 자신의 브랜치를 새로운 기능 브랜치로 병합합니다.
  5. 자신의 기능 브랜치를 메인 브랜치로 병합하기 위한 새 병합 요청을 엽니다.
  6. 커뮤니티 MR에서 자신의 MR을 연결하고 ~"커뮤니티 기여"로 라벨을 지정합니다.
  7. 필요한 최종 조정을 수행하고 기여자에게 변경 사항을 검토할 기회와 기여자의 콘텐츠가 메인 브랜치로 병합되고 있다는 사실을 알리기 위해 핑을 보냅니다.
  8. 콘텐츠가 모든 병합 요청 지침을 준수하는지 확인합니다.
  9. 일반적인 병합 요청과 마찬가지로 정기적인 리뷰 프로세스를 따릅니다.

올바른 균형

코드 검토 중 가장 어려운 일 중 하나는 저자가 작성한 코드에 평가자가 얼마나 개입해야 하는지에 대한 올바른 균형을 찾는 것입니다.

  • 올바른 균형을 찾는 방법을 배우는 데에는 시간이 걸립니다. 그것이 바로 일정 기간 동안 병합 요청을 검토한 후 유지자가 되는 리뷰어들이 있는 이유입니다.
  • 버그를 찾는 것은 중요하지만 좋은 디자인을 고민하는 것 또한 중요합니다. 추상화 및 좋은 설계를 구축하는 것이 복잡성을 숨기고 미래의 변경을 쉽게 만드는 것을 가능하게 합니다.
  • 코드 스타일의 강화 및 향상은 주로 자동화를 통해 이루어져야 하며, 리뷰 코멘트를 통해 하는 것이 아닙니다.
  • 저자에게 디자인을 변경하도록 요청하는 것은 기여한 코드를 완전히 다시 작성하는 것을 의미합니다. 일반적으로 그것을 하기 전에 다른 유지자나 리뷰어에게 물어보는 것이 좋지만, 중요하다고 믿을 때에는 용기를 내어 진행하십시오.
  • 반복의 관점에서, 귀하의 검토 제안이 비차단 변경이거나 개인적인 선호도(문서화되거나 합의된 요구 사항이 아닌 경우)인 경우, 저자에게 돌려보내기 전에 병합 요청을 승인하는 것을 고려하십시오. 이는 저자가 동의한다면 귀하의 제안을 구현할 수 있게 하거나 유지자에게 즉시 전달할 수 있게 합니다. 이로써 총 병합 시간을 줄일 수 있습니다.
  • 일을 올바르게 하는 것과 지금 옳게 하는 것 사이에 차이가 있습니다. 이상적으로는 전자를 해야 하지만, 현실에서는 후자가 필요합니다. 가능한 빨리 릴리스해야 하는 보안 수정이 좋은 예입니다. 긴급한 수정인데 저자에게 병합 요청에서 주요한 리팩터링을 하도록 요청하는 것은 피해야 합니다.
  • 오늘 일을 잘 하는 것은 훌륭한 것을 미래에 완벽하게 하는 것보다 일반적으로 더 나은 결과를 가져옵니다. 오늘 쓸데없는 것을 출시하는 것은 내일 훌륭하게 하는 것보다 좋지 않습니다. 올바른 균형을 찾지 못할 때에는 다른 사람에게 의견을 물어보십시오.

GitLab-특정 사항

GitLab은 많은 곳에서 사용됩니다. 많은 사용자가 Omnibus 패키지를 사용하지만, 일부는 Docker 이미지를 사용하고, 일부는 원본에서 설치하며, 다른 설치 방법도 있습니다. GitLab.com 자체가 큰 Enterprise Edition 인스턴스입니다. 이에는 몇 가지 영향이 있습니다:

  1. 쿼리 변경 사항은 GitLab.com 규모에서 성능이 나빠지지 않는지 테스트해야 합니다:
    1. 대량의 데이터를 로컬에서 생성하는 것이 도움이 될 수 있습니다.
    2. GitLab.com에서 쿼리 계획을 요청하는 것이 가장 신뢰할 수 있는 유효성 검사 방법입니다.
  2. 데이터베이스 마이그레이션은:
    1. 되돌릴 수 있어야 합니다.
    2. GitLab.com 규모에서 성능이 좋아야 합니다. 확실하지 않다면 관리자에게 마이그레이션을 테스트하라고 요청하세요.
    3. 올바르게 분류되어야 합니다:
  3. Sidekiq 워커하위 호환성을 유지해야 합니다:
    1. Sidekiq 대기열이 배포되기 전에 비워지지 않으므로 이전 버전의 GitLab에서 대기 중인 워커가 있습니다.
    2. 메서드 시그니처를 변경해야 하는 경우 같은 기능을 두 버전에 걸쳐 변경하고, 첫 번째 변경에서는 이전 및 새 인수를 모두 허용하세요.
    3. 마찬가지로 워커를 제거해야 하는 경우, 첫 번째로 예약되지 않도록 하고, 그 다음에 제거하세요. 이렇게 하면 기존 작업이 실행됩니다.
    4. 기억하세요, 모든 인스턴스가 모든 중간 버전으로 업그레이드되지는 않습니다(어떤 사람은 X.1.0에서 X.10.0으로, 더 큰 업그레이드도 해보겠죠!), 그러므로 가능한 경우 이전 형식을 수용하는 것이 좋습니다.
  4. 캐시된 값은 버전 간에 유지될 수 있습니다. 캐시 값이 반환하는 유형을 변경하는 경우(예: 문자열 또는 nil에서 배열로), 동시에 캐시 키를 변경하세요.
  5. 설정마지막 수단으로 추가되어야 합니다. GitLab Rails에 새 설정을 추가하는 방법은 여기를 참조하세요.
  6. 파일 시스템 액세스클라우드 네이티브 아키텍처에서 불가능합니다. 수행해야 하는 파일 저장소에 대한 객체 저장소 지원이 필요합니다. 자세한 내용은 업로드 문서를 참조하세요.

고객 중요 병합 요청

병합 요청은 비즈니스에 상당한 이점이 있을 경우 고객 중요 우선 순위로 고려될 수 있습니다.

고객 중요 병합 요청의 속성:

  • 개발에서 고위 관리자 이상이 병합 요청이 고객 중요 사항인지를 승인하여야 합니다. 또는 직속 보고에게 승인을 요청할 경우, 승인이 가능합니다.
  • DRI는 병합 요청에 customer-critical-merge-request 라벨을 적용해야 합니다.
  • 고객 중요 병합 요청과 관련된 리뷰어 및 유지보수자는 이 결정이 내려진 즉시 참여해야 합니다.
  • 고객 중요 병합 요청에 관련된 업무를 우선순위를 정하여 관련 인원이 해당 업무에 집중할 수 있도록 해야 합니다.
  • GitLab의 가치와 프로세스를 준수해야 하며, 특히 가족 및 친구 우선/업무 후, 완료 정의, 반복 및 준비되었을 때 릴리스에 특히 유의해야 합니다.
  • 고객 중요 병합 요청은 보안을 감소시키거나 데이터 손실, 가용성 저하, 기존 기능 손상을 일으키지 않고, 기술적 결정에 우선순위를 붙이는 과정에 따라야 합니다.
  • 고객 중요 요청에 대해서는 병합 여부가 결정된 이후, 병합 요청의 경과 시간을 줄일 수 있는지 여부를 고려하여 (병합 요청 코멘트)비동기적으로 조정하는 것도 _고려_되며, 이는 효율성을 희생할 수도 있는 것입니다.
  • 고객 중요 병합 요청이 병합된 후, 향후 고객 중요 병합 요청의 빈도를 줄이는 것이 목적인 회고를 마쳐야 합니다.

예시

코드 리뷰는 새 기여자들을 놀라게 할 수 있습니다. 여기 몇 가지 코드 리뷰 예시가 있어서, 기대할 것을 파악하는 데 도움이 될 것입니다.

DiffNote 수정하여 설계에 재사용”: 새 줄바꿈 주변의 작은 지적부터 설계 버전에 대한 이유에 이르기까지 모든 내용을 담고 있었으며, 만약 특정 파일의 이전 버전이 없는 경우에는 어떻게 비교해야 하는지에 대해 논의되었습니다 (부모 vs. 빈 sha vs. 빈 트리).

“여러 줄 제안 지원”: MR 자체는 FE와 BE 간의 협업으로 이루어져 있으며, 리뷰어를 위한 작성자의 코멘트에 대한 문서화도 포함되어 있습니다. 일부 작은 지적, 정보 요청에 관한 몇 가지 질문이 있으며, 마지막에는 보안 취약점이 있습니다.

“프로젝트당 여러 저장소 허용”: ZJ는 다른 프로젝트 (workhorse)에 영향을 줄 수 있는 내용에 언급하며, 일관성을 위한 몇 가지 개선을 제안했습니다. 그리고 James의 코멘트는 전반적인 코드 품질에 도움이 되었고 (위임, &. 등의 요소를 사용), 코드를 더 견고하게 만드는 데 도움이 되었습니다.

“병합 요청에 대한 여러 담당자 지원”: 코드베이스의 여러 부분에 걸친 MR에 대한 협업의 좋은 예시입니다. Nick은 흥미로운 예외 사례를 지적했고, James Lopez도 가져온/내보낸 기능에 대한 우려를 표명했습니다.

크레딧

대부분은 thoughtbot 코드 리뷰 안내를 기반으로 합니다.