코드 리뷰 지침

이 안내서에는 코드 리뷰를 수행하고 코드 리뷰를 받는 데 도움이 되는 조언과 모범 사례가 담겨 있습니다.

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

병합 요청이 검토되고 승인되고 병합되는 방법

시작하기 전에:

기여 승인 기준을 숙지하세요.
도움이 필요한 경우(예: 첫 번째 병합 요청인 경우) 병합 요청 코치 중 한 명에게 문의하세요.

코드를 검토해야 할 때 어떤 이에게 코드를 검토받으세요. 검토자가 될 수 있는 사람은 귀하의 그룹 또는 팀원이거나 도메인 전문가일 수 있습니다. 검토자는:

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

만약 병합 요청이 작고 검토하기 간편하다면 검토자 단계를 건너뛸 수 있고 직접적으로 유지자(maintainer)한테 물어볼 수 있습니다.

“작고 간편하다”에 대한 기준은 모호합니다. 여기 작고 간단한 변경 사항의 예시가 있습니다:

오탈자 수정 또는 소규모 문구 변경 (example).
어떤 행위나 데이터를 변경하지 않는 소규모 리팩터링.
1개월 이상 기본으로 활성화된 기능 플래그에 대한 참조 제거.
사용되지 않는 메서드나 클래스 제거.
5줄 이내의 코드를 변경해야 하는 잘 이해되는 논리 변경 사항.

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

보안 스캔 또는 코멘트에 도움이 필요한 경우, @gitlab-com/gl-security/appsec에게 어시스턴스를 요청하세요.

검토자들은 사이드바의 검토자 기능성을 사용합니다. 검토자는 병합 요청 승인 추가로로 승인을 추가할 수 있습니다.

귀하의 병합 요청이 어떤 영역에 영향을 미치는 지에 따라 하나 이상의 유지자(maintainers)의 승인을 받아야 합니다. 승인 버튼은 병합 요청 위젯에 있습니다.

귀하의 병합 요청을 병합하기 위해서는 유지자가 필요합니다. 하나 이상의 승인이 필요한 경우, 마지막으로 검토하고 승인한 유지자가 병합을 수행합니다.

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

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

도메인 전문가

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

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

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

특정 스테이지/그룹(예: 생성: 소스 코드)에서 작업하는 팀 구성원은 그들이 작업한 앱 영역의 도메인 전문가로 간주됩니다.
특정 기능(예: 검색)에서 작업하는 팀 구성원은 해당 기능의 도메인 전문가로 간주됩니다.

우리는 코드 리뷰를 위해 기본값으로 팀원들에게 도메인 전문가로서의 리뷰를 할당합니다. 디자인 리뷰는 디자이너 용량 제한으로 인해, 제품 디자이너가 지원하지 않는 영역은 커뮤니티 기여인 경우에만 디자인 리뷰가 필요하지 않습니다. 적합한 도메인 전문가를 찾을 수 없는 경우, 병합 요청을 리뷰할 수 있도록 아무 팀원이나 선택하거나 리뷰자 룰렛 권장사항을 따를 수 있습니다(디자인 리뷰에 대한 자세한 내용은 위를 참조하세요). 해당 사람이 휴가 중인지 다시 한 번 확인하세요.

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

병합 요청 승인 위젯에서 승인 가능한 승인자 보기를 선택하세요. 이 위젯에는 코드베이스 영역별 권장 및 필수 승인이 표시됩니다. 이러한 규칙은 코드 소유자(Code Owners)에 정의되어 있습니다.
병합 요청과 관련된 단계 또는 그룹에 대한 팀 구성원 목록을 볼 수 있습니다.
엔지니어링 프로젝트 페이지나 GitLab 팀 페이지에서 팀원의 도메인 전문성을 볼 수 있습니다. 도메인은 스스로 식별되므로 귀하의 병합 요청에 대한 변경 사항을 매핑하기 위해 판단을 사용하세요.
병합 요청의 파일에 기여한 팀 멤버를 찾아보세요. git log <file>을 실행하여 로그를 볼 수 있습니다.
파일을 검토한 팀 멤버를 찾아보세요. 관련 병합 요청은 다음과 같이 찾을 수 있습니다.
1. git log <file>을 사용하여 커밋 SHA를 가져옵니다.
2. https://gitlab.com/gitlab-org/gitlab/-/commit/<SHA>로 이동합니다.
3. 커밋을위한 표시된 관련 병합 요청을 선택합니다.

리뷰어 룰렛

참고:

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

Danger bot은 당신의 병합 요청이 영향을 줄 것으로 보이는 코드베이스 각 영역에 대해 리뷰어와 유지보수자를 무작위로 선택합니다. 개발자 리뷰어를 권장하며, 다른 사람이 더 적합하다고 생각된다면 재정의해야 합니다.

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

제품 디자이너가 포함된 팀의 MR에서만 UX 리뷰를 수행합니다. 이러한 팀의 사용자 중심 변경 사항은 기능 플래그에 의해 가려져 있더라도 UX 리뷰가 필요합니다. 권장된 UX 리뷰어가 제안된 대로 따르세요.

리뷰어와 유지보수자는 다음과 같은 동작을 하는 공학 프로젝트 페이지에 나열된 사람들 중에서 선택합니다.

Slack 또는 GitLab 상태가 다음 문자열 중 하나를 포함하는 사람을 선택하지 않습니다:
- OOO, PTO, 육아 휴가, 가족과의 시간, 또는 컨퍼런스
- 다음 범주 중 하나의 이모지를 가진 사람:
  - 휴가 - 🌴 :palm_tree:, 🏖️ :beach:, ⛱ :beach_umbrella:, 🏖 :beach_with_umbrella:, 🌞 :sun_with_face:, 🎡 :ferris_wheel:, 🏙 :cityscape:
  - 병가 - 🌡️ :thermometer:, 🤒 :face_with_thermometer:
이미 선택된 리뷰 수가 선택한 “리뷰 제한”과 동일하거나 그보다 많은 사람을 선택하지 않습니다. 리뷰 제한은 한 번에 처리할 수 있는 리뷰 최대 수입니다. Slack 또는 GitLab 상태를 사용하여 리뷰 제한을 설정하세요:
- 2️⃣ - :two:
- 3️⃣ - :three:
- 4️⃣ - :four:
- 5️⃣ - :five: 최소 리뷰 제한은 2️⃣입니다. 완전히 리뷰를 해제할 수 없는 이유는 이 이슈에서 논의되었습니다. 보안 그룹에 속한 프로젝트의 기본 브랜치를 타겟팅하지 않는 병합 요청의 리뷰 요청은 세지 않습니다. 이러한 MR은 일반적으로 백포트이며 유지보수자나 리뷰어는 보통 이에 많은 시간을 할애하지 않습니다.
동일한 브랜치 이름에 항상 동일한 리뷰어와 유지보수자를 선택합니다(1번에서 언급한 out-of-office(OOO) 상태 변화를 제외하고). 백포트 브랜치에서 안정적으로 유지될 수 있도록 앞에 있는 ce- 및 ee-와 뒤에 있는 -ce 및 -ee를 제거합니다.
Slack 또는 GitLab 상태 이모지가 Ⓜ :m:인 사람은 해당 프로젝트에서 유지보수자로만 제안됩니다.

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

지난 7일과 30일 동안의 할당 이벤트.
현재 사람 당 할당된 병합 요청.
다른 기준에 따른 정렬.
수동 리뷰어 룰렛.
현지 시간 정보.

더 많은 정보는 룰렛 README를 참고하세요.

승인 지침

유지보수자의 책임에 대한 섹션에 설명된 대로, 병합 요청을 승인하고 병합하는 것이 권장되는 도메인 전문가에게 승인받기를 권장합니다. 첫 번째 리뷰어의 선택적 승인은 여기에 포함되지 않습니다. 그러나 당신의 병합 요청은 개요 섹션에 설명된 대로 유지보수자에게 전달되기 전에 리뷰어에 의해 검토되어야 합니다.

당신의 병합 요청이 다음을 포함하는 경우	승인은 다음 중 하나에 의해 이루어져야 합니다
`~backend` 변경 사항 ¹	백엔드 유지보수자.
`~database` 마이그레이션 또는 고비용 쿼리 변경 ²	데이터베이스 유지보수자. 자세한 내용은 데이터베이스 검토 지침을 참조하세요.
`~workhorse` 변경 사항	Workhorse 유지보수자.
`~frontend` 변경 사항 ¹	프론트엔드 유지보수자.
`~UX` 사용자 중심 변경 사항 ³	제품 디자이너. 자세한 내용은 디자인 및 사용자 인터페이스 지침을 참조하세요.
새로운 JavaScript 라이브러리 추가 ¹	- 프론트엔드 디자인 시스템 구성원, 라이브러리가 번들 크기를 크게 늘릴 경우. - 사용 중인 라이센스가 GitLab에서 승인되지 않은 경우 법률 부서 구성원. 라이센스 호환성에 대한 자세한 정보는 GitLab 라이센스 및 호환성 문서에서 확인할 수 있습니다.
새로운 종속성이나 파일 시스템 변경	- 분배 팀 구성원. 자세한 내용은 분배 팀과의 작업 방법을 참조하세요. - RubyGems의 경우, AppSec 리뷰를 요청하세요.
`~documentation` 또는 `~UI 텍스트` 변경	적절한 DevOps 스테이지 그룹의 과제에 대한 과제를 기반으로 기술 작성자로부터 지시를 받아야 합니다.
개발 지침 변경	검토 프로세스를 따르고, 해당 승인을 받으세요.
종단 간 및 비종단 간 변경 사항 ⁴	품질 유지보수자.
오직 종단 간 변경 사항 ⁴ 또는 MR 작성자가 품질 유지보수자인 경우	품질 유지보수자.
새로운 또는 업데이트된 응용 프로그램 한도	제품 관리자.
분석 계기 (텔레메트리 또는 분석) 변경	분석 계기 엔지니어.
기능 사양의 추가 또는 변경	품질 유지보수자 또는 품질 리뷰어.
GitLab에 새로운 서비스 추가 (Puma, Sidekiq, Gitaly 등)	제품 관리자. 자세한 내용은 GitLab에 서비스 구성 요소를 추가하는 프로세스를 참조하세요.
인증과 관련된 변경 사항	관리:인증. 자세한 내용은 그룹 페이지의 코드 리뷰 섹션을 확인하세요. 팀에서 리뷰가 필요한 파일에 대한 패턴은 `CODEOWNERS` 파일의 `Authentication` 섹션에 나열되어 있으며, 해당 팀은 이러한 파일을 수정하는 모든 병합 요청의 승인자 섹션에 나열됩니다.
사용자 정의 역할 또는 정책과 관련된 변경 사항	관리:인증 엔지니어.

CODEOWNERS 승인

특정 그룹의 승인이 필수인 경우도 있는 병합 요청이 있습니다. 정의에 대해서는 .gitlab/CODEOWNERS를 참조하세요.

.gitlab/CODEOWNERS의 필수 섹션은 다음과 같은 경우에만 제한해야 합니다.

규정 준수
가용성
보안

필수 섹션을 추가할 때는 새로운 필수 섹션이 병합 요청 비율에 미치는 영향을 추적해야 합니다. 좋은 예제는 Verify issue에서 확인할 수 있습니다.

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

또한, 현재 단일 모노리스 구조는 병합 요청을 통해 보이는 듯하지 않은 부분에 영향을 줄 수 있다. 여러 필수 승인은 이러한 병합 요청이 작성자가 승인을 요청해야 한다는 것을 의미하며, 효율적이지 않습니다.

이를 개선하기 위한 노력은 다음과 같습니다.

승인 체크리스트

이 체크리스트는 병합 요청(MR)의 작성자, 리뷰어 및 유지 관리자가 변경 사항을 품질, 성능, 신뢰성, 보안, 가시성, 유지보수 가능성에 대한 고위험 위험을 분석했는지 확인하는 데 도움을 주는 것입니다.

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

품질

추가 품질 지침에 대해서는 테스트 엔지니어링 프로세스를 참조하세요.

이 MR을 코드 리뷰 지침에 따라 자체 검토했습니다.
이 변경이 영향을 주는 코드에 대해 자동화된 테스트(테스트 가이드)가 사용자에게 매우 중요한 기능을 유효성 검사한다고 믿습니다(모든 테스트 수준을 고려함).
기존의 자동화된 테스트가 위의 기능을 포함하지 않는 경우, 해당하는 추가 테스트를 추가했거나 이러한 자동화 테스트 누락을 설명하는 이슈를 추가하고 이 MR에 연결했다고 가정합니다.
이 변경 사항이 GitLab.com 호스팅 고객 및 자체 관리 고객에 미치는 기술적 영향을 고려했습니다.
이 변경 사항이 시스템의 frontend, backend 및 데이터베이스 부분에 미치는 영향을 고려하고 ~ux, ~frontend, ~backend, ~database 레이블을 적용했습니다.
이 MR을 모든 지원되는 브라우저에서 테스트했거나, 이러한 테스트가 필요하지 않다고 결정했습니다.
이 변경이 업데이트를 통해 하위 호환성을 유지하는지 확인했거나, 해당 사항이 적용되지 않았다고 가정했습니다.
EE 콘텐츠를 FOSS에서 제대로 분리했거나 이 MR이 FOSS 전용입니다.
- EE 코드는 어디에 넣어야 합니까?
이 MR이 EE 및 FOSS에 서로 다른 방식으로 영향을 줄 수 있는지 고려했으며 FOSS 컨텍스트에서 CI 파이프라인을 실행해야 한다고 가정했습니다.
기존 데이터가 놀랍도록 다양할 수 있다는 점을 고려했습니다. 예를 들어, 새로운 모델 유효성 확인은 기존 레코드를 망가뜨릴 수 있습니다. 기존 데이터가 유효성 검사를 통과할 것이라고 확인하지 않았다면 새로운 데이터의 유효성을 필수로 만들기보다는 선택 사항으로 만들 것을 고려하세요.
테스트가 경고와 함께 통과하며 실패한 작업에 Flaky test '<path/to/test>' was found in the list of files changed by this MR. 텍스트가 포함된 경우, 해당 테스트를 수정하거나 왜 이러한 불안정한 테스트를 무시해야 하는지 설명한 증거를 제공했습니다.

성능, 신뢰성 및 가용성

이 MR이 성능에 해를 끼치지 않을 것이라고 확신합니다. 또는 성능 영향을 평가하기 위해 리뷰어에게 도움을 요청했습니다.(병합 요청 성능 가이드라인)
MR 설명에 데이터베이스 관련 변경 사항에 대한 정보를 추가했거나 불필요하다고 결정했습니다.
- 이 MR에 데이터베이스 관련 변경 사항이 있는지 확인합니까? (database_review.md)
이 변경의 가용성과 신뢰성에 대한 위험을 고려했습니다.
미래 예측 성장을 기반으로 확장 가능성에 대한 위험을 고려했습니다.
이 변경이 평균 고객보다 훨씬 더 많은 데이터를 가지고 있을 수 있는 대규모 고객에게 미치는 성능, 신뢰성 및 가용성 영향을 고려했습니다.
이 변경이 최소 시스템에서 GitLab을 실행할 수 있는 고객에게 미치는 성능, 신뢰성 및 가용성 영향을 고려했습니다.

가시성 계측

가시성을 통해 디버깅 및 적극적인 성능 향상이 가능하도록 충분한 계측을 포함했습니다. 예제를 참조하여 기능 플래그, 로깅 및 계측 추가를 확인했습니다.

문서

변경 사항을 반영하는 변경로그 트레일러를 포함했거나 불필요하다고 결정했습니다.
- 이 MR에 변경 로그가 필요합니까?
문서를 추가/업데이트했거나 이 MR에 대한 문서 변경이 불필요하다고 결정했습니다.
- 문서가 필요합니까?

보안

이 MR에 자격증명이나 토큰, 인가 및 인증 방법 등에 대한 처리 또는 저장 변경 사항이 포함되어 있다면 ~security 레이블을 추가하고 @gitlab-com/gl-security/appsec를 언급했습니다.
보안 검토 지침에 설명된 것과 같은 보안 검토를 요청해야 하는 이 변경에 대해 내부 응용 프로그램 보안 검토에 대한 문서를 검토했습니다.
MR을 차단한 보안 검사 결과가 있는 경우(병합 요청 승인 정책으로 인한):
- 실제 양성 결과에 대해서는 MR이 병합되기 전에 수정해야 합니다. 이로 인해 병합 요청 승인 정책으로 인한 AppSec 승인이 제거됩니다.
- 거짓 양성 결과에 대해서는 위험 수용에 대해 논의해야 하며 의문사항이나 무시해야 할 사항에 대해서는 @gitlab-com/gl-security/appsec를 언급하세요.

배포

이 변경 사항은 고위험일 수 있기 때문에 이 변경 사항에 대한 기능 플래그 사용을 고려했습니다.
기능 플래그를 사용하는 경우, 프로덕션에서 테스트하기 전에 스테이징에서 변경 사항을 테스트하고, 모든 고객에게 전파하기 전에 프로덕션의 일부 고객에게 롤아웃을 고려했으며, 이에 대해 계획을 세웠습니다.
- 기능 플래그 사용 시기
기본 설정 또는 새로운 설정 변경에 대한 정의가 완료되었거나 이를 필요하지 않다고 결정했음을 인프라스트럭처 부서에 알렸습니다. (완료 정의)

컴플라이언스

올바른 MR 타입 라벨이 적용되었는지 확인했습니다.

병합 요청 작성자의 책임

최적의 솔루션을 찾고 구현하는 책임은 병합 요청 작성자에게 있습니다. 작성자 또는 직접 책임 있는 개인 (DRI)은 코드 검토 수명주기 동안에 계속해서 이 병합 요청에 대한 담당자로서 지정됩니다. 자신을 담당자로 설정할 수 없는 경우, 리뷰어에게 이를 요청하세요.

유지보수자에게 승인 및 병합을 요청하기 전에 다음 사항에 대해 확신해야 합니다.

그것이 의도한 대로 문제를 해결합니다.
가장 적절한 방법으로 그것을 수행합니다.
모든 요구 사항이 충족되었습니다.
남아 있는 버그, 논리적 문제, 누락된 엣지 케이스, 또는 알려진 취약점이 없습니다.

리뷰어와의 불필요한 오고가를 피하고 자신의 병합 요청을 심사할 때 가장 좋은 방법은 코드 검토 지침을 따라 자체 검토하는 것입니다. 이 자체 검토 중에, 의사를 결정하거나 교환해야 하는 상황 또는 맥락적 설명이 리뷰어가 코드를 보다 쉽게 이해하는 데 도움이 될 수 있는 경우 MR에 주석을 추가하십시오.

자신의 솔루션에 대한 필요한 수준의 신뢰를 얻기 위해 작성자는 조사 및 구현 프로세스에서 적절하게 다른 사람들을 참여시킬 것으로 예상됩니다.

작성자는 다른 솔루션을 논의하거나 구현을 검토하기 위해 도메인 전문가에게 연락을 취하는 것이 권장되며, 혼란을 해소하거나 최종 결과가 의도한 것과 일치하는지 확인하기 위해 제품 관리자 및 사용자 경험 디자이너에게 연락을 취하거나 데이터 모델 또는 특정 쿼리에 대한 입력을 얻기 위해 데이터베이스 전문가에게 연락을 취하거나, 솔루션에 대한 심층적인 검토를 얻기 위해 다른 개발자에게 연락을 취합니다.

특정 기능을 제공하기 위해 여러 병합 요청이 필요할 것으로 예상된다면(예: 개념 증명을 생성했고 해당 기능이 10개 이상의 병합 요청으로 구성될 것으로 명확하다면), 해당 기능의 필요한 이해를 가지고 있는 리뷰어 및 유지자를 식별하는 것이 좋습니다(해당 컨텍스트를 공유합니다). 그런 다음 모든 병합 요청을 이러한 리뷰어에게 지시하십시오. 이러한 리뷰어를 찾는 가장 좋은 DRI는 EM 또는 스태프 엔지니어입니다. 동일한 컨텍스트로 여러 병합 요청에 대한 안정적인 리뷰어 상대방을 보유하는 것은 효율성을 향상시킵니다.

병합 요청이 여러 도메인에 영향을 미치는 경우(예: Dynamic Analysis 및 GraphQL), 각 도메인 전문가로부터 검토를 요청하십시오.

작성자가 병합 요청이 도메인 전문가의 의견이 필요한지 확실하지 않다면, 그것은 필요성이 있다는 것을 의미합니다. 이것이 없으면 솔루션에 대한 필요한 수준의 신뢰가 없을 가능성이 높습니다.

리뷰를 시작하기 전에 작성자는 병합 요청의 차이에 대한 코멘트를 제출하도록 요청됩니다. 또한 게요 사항뿐만 아니라 더 많은 설명이나 관심이 필요한 내용에 대해서도 리뷰어에게 경고하는데 주의해야 합니다. 다음 내용 중 주석이 필요할 수 있는 콘텐츠의 예는 다음과 같습니다.

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

검토자가 솔루션을 확인하기 위해 필요한 프로젝트, 스니펫 또는 기타 자산이 있다면 검토 요청을 하기 전에 해당 자산에 액세스할 수 있도록 하십시오.

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

각 리뷰어가 제공해야 하는 리뷰의 유형을 MR에 코멘트로 추가합니다.
- 예를 들어, MR이 데이터베이스 쿼리를 변경하고 백엔드 코드를 업데이트한다면, MR 작성자는 먼저 ~backend 리뷰 및 ~database 리뷰가 필요합니다. 리뷰어를 지정하는 동안, 작성자는 각 리뷰어가 어떤 도메인을 검토해야 하는지를 알려주는 코멘트를 MR에 추가합니다.
- 많은 GitLab 팀 멤버가 하나 이상의 영역에 도메인 전문가이므로, 이러한 유형의 코멘트가 없으면 때로는 리뷰하는데 어떤 유형의 리뷰를 제공해야 하는지 명확하지 않을 때가 있습니다.
- MR 검토 유형에 대한 명시성은 MR 작성자에게 효율적이며, 그들은 원하는 유형의 리뷰를 받을 수 있고, MR 리뷰어에게는 즉시 제공해야 하는 리뷰의 유형을 알게 돼 효율적입니다.
- 예시 1
- 예시 2

피해야 할 것:

(상기 참조)를 직접 소스 코드에 추가하지 마십시오. TODO 코멘트를 추가하려면 관련 이슈에 대한 링크를 포함하세요.
코드가 무엇을 하는지에 대해 설명하는 주석 추가. 비-TODO 코멘트를 추가해야 하는 경우, 그것은 왜 하는지를 설명해야 합니다.
실패한 테스트가 있는 병합 요청에 유지자 검토를 요청하지 마십시오. 테스트가 실패하고 검토를 요청해야 하는 경우, 설명이 있는 코멘트를 추가해야 합니다.
이메일이나 Slack을 통해 유지자를 지나치게 언급하지 마십시오 (유지자가 Slack을 통해 연락할 수 있는 경우). 병합 요청에 리뷰어를 추가할 수 없는 경우, 코멘트에서 유지자를 @ 언급하는 것은 허용되며, 기타 모든 경우에 리뷰어를 추가하는 것이 충분합니다.

이것은 리뷰어의 시간을 절약하고, 작성자가 실수를 빨리 포착하는 데 도움이 됩니다.

리뷰어의 책임

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

리뷰 응답 SLO 내에서 할당된 병합 요청을 검토할 수 없는 경우:

작성자에게 사용할 수 없음을 알립니다.
GitLab Review Workload Dashboard를 사용하여 새 리뷰어를 선택합니다.
새 리뷰어를 병합 요청에 할당합니다.

이는 행동의 편향을 보여주며 효율적인 MR(Merge Request) 리뷰 진행을 보장합니다.

다음과 같이 설명하는 댓글을 추가합니다.

안녕하세요 <@mr-author>, 리뷰를 할 수 없어요. 그러나 이 프로젝트를 위해 [룰렛 휠을 돌렸습니다.](https://gitlab-org.gitlab.io/gitlab-roulette/) 그리고 <@new-reviewer>(이)가 선택되었습니다.

@new-reviewer, 시간이 되시면 이 병합 요청을 리뷰해주시겠어요? 시간이 안 되시면 다시 [룰렛 휠을 돌리고](https://gitlab-org.gitlab.io/gitlab-roulette/) 새 리뷰어를 선택하고 할당해주십시오, 감사합니다.

/assign_reviewer <@new-reviewer>
/unassign_reviewer me

병합 요청 검토를 철저히 수행합니다.

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

일부 병합 요청은 도메인 전문가의 도움이 필요할 수 있습니다. 리뷰어는 해당 영역의 도메인 전문가가 아닌 경우 다음 중 하나를 수행할 수 있습니다:

병합 요청을 검토하고 다른 리뷰어 또는 유지관리자를 연결시킵니다. 이 전문가 는 다른 리뷰어 또는 유지관리자가 될 수 있습니다.
더 적합한 리뷰어에게 리뷰를 넘깁니다.
도메인 전문가가 없는 경우 최선을 다해서 리뷰합니다.

병합 요청을 다음과 같이 작게 분할하도록 작성자에게 안내해야 합니다. 그렇다면:

너무 큽니다.
둘 이상의 문제를 해결합니다.
둘 이상의 기능을 구현합니다.
추가 위험으로 이어지는 높은 복잡성을 가집니다.

작성자는 현재 유지관리자와 리뷰어가 분할된 MR를 검토하거나 새로운 유지관리자 및 리뷰어 그룹을 요청할 수 있습니다.

모든 요구 사항을 충족하는 것을 확신할 때:

승인을 선택합니다.
작성자에게 @을 언급하여 to-do 알림을 생성하고, 병합 요청이 리뷰되고 승인되었음을 알립니다.
유지관리자에게 리뷰를 요청합니다. 도메인 전문성을 갖춘 유지관리자에게 기본 요청하며, 사용 가능한 경우 또는 병합 요청이 도메인 전문가의 리뷰가 필요하지 않다고 생각되는 경우 리뷰어 룰렛 제안을 따라 진행할 수 있습니다.
리뷰어로서 제거합니다.

유지관리자의 책임

유지관리자는 GitLab 코드베이스의 전반적인 건강, 품질 및 일관성에 책임이 있습니다. 도메인과 제품 영역에 걸쳐서 유지관리자의 리뷰는 주로 전체적인 아키텍처, 코드 구성, 관심의 분리, 테스트, DRYness, 일관성 및 가독성과 같은 것에 중점을 둡니다.

결국 유지관리자의 작업은 특정 도메인의 지식이 아니라 전체적인 GitLab 코드베이스에 대한 지식에만 의존하기 때문에, 그들은 어떤 팀에서든 제품 영역에서든 모든 병합 요청을 검토, 승인 및 병합할 수 있습니다.

유지관리자는 병합 요청의 수용 기준이 합리적으로 충족되었는지 확인하는 DRI(책임있는 담당자)입니다. 일반적으로 품질은 모두의 책임이지만, MR(Merge Request)의 유지관리자는 보장에 대한 책임이 있습니다. MR에서의 기술적 빚 생성을 피하도록 해야 합니다.

유지관리자는 MR가 상당히 중요하다고 판단되거나 도메인 전문가가 필요한 경우, 다른 리뷰어 또는 유지관리자에게 리뷰를 요청할 수 있습니다. 다음은 리뷰 중에 유지관리자가 이렇게 하는 몇 가지 예시입니다.

유지관리자는 병합 전에 선택한 솔루션의 구체적인 내용도 검토하지만, 이들은 반드시 도메인 전문가가 아니기 때문에 합리적인 시간 투자 없이는 그렇게 할 수 없을 수 있습니다. 이 경우에는 작성자와 이전의 리뷰어들의 판단을 우선시하고 본인의 주요 업무에 집중합니다.

혹여라도 개발자가 리뷰어로 참여하는 경우 유지관리자로서 최종 승인 및 병합을 수행하지 않는 것이 좋습니다.

유지관리자는 병합하기 전에 필요한 승인자가 승인했는지 확인해야 합니다. 다른 사람의 추가적인 승인을 기다리는 경우 리뷰어 자격을 제거한 후 작성자에게 @을 언급하고 그 이유를 설명하는데 주의하세요. 코드를 병합하는 경우 리뷰어로 남아야 합니다.

특정 병합 요청은 안정된 브랜치를 대상으로 할 수 있습니다. 이러한 요청을 처리하는 방법에 대한 개요는 패치 릴리스 작업 가이드를 참조하십시오.

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

리뷰어 기능의 도그후딩

2021년 3월 18일에 리뷰어 기능을 효율적이고 일관되게 활용하기 위한 업데이트된 프로세스가 구현되었습니다.

여기에 변경 내용에 대한 요약이 있으며, 위의 섹션에도 반영되어 있습니다.

병합 요청 작성자 및 DRI(책임있는 담당자)는 Assignees로 남습니다.
작성자는 Reviewers로 사용자를 할당하여 리뷰를 요청합니다.
리뷰어는 리뷰와 승인을 마치면 자신을 Unassign합니다.
MR를 병합하는 마지막 승인자는 리뷰어로서 할당된 상태로 남습니다.

최선의 실천법

모두

친절하게 대해주세요.
많은 프로그래밍 결정이 의견입니다. 트레이드오프를 논의하고, 당신이 선호하는 대로 빨리 해결하세요.
질문하세요. 명령하지 마세요. (“이것을 :user_id로 명명하는 것에 대해 어떻게 생각하세요?”)
명확한 답변을 요청하세요. (“이해가 안 가요. 명확히 설명해주실 수 있나요?”)
코드를 선택적으로 소유하지 마세요. (“내 것”, “아닌 것”, “네 것”)
개인적인 특성을 나타내는 것으로 보일 수 있는 용어를 사용하지 마세요. (“멍청한”, “바보 같은”). 모든 사람이 지적이고 선량하다고 가정하세요.
명시적으로 표현하세요. 온라인에서 여러분의 의도를 이해하지 못할 수 있습니다.
겸손하세요. (“잘 모르겠어요 - 찾아보죠.”)
과장하지 마세요. (“항상”, “절대”, “끝없이”, “아무것도”)
빈정대 사용에 주의하세요. 저희가 하는 모든 것은 공개됩니다. 당신과 오랜 동료에게는 유쾌한 농담으로 보일 수 있는 것이 새로 프로젝트에 참여한 사람에게는 무례하고 환영받지 못할 수 있습니다.
“이해하지 못했습니다” 또는 “대안 솔루션:”과 같은 댓글이 너무 많다면 일대일 대화나 비디오 통화를 고려하세요. 일대일 토론의 요약 댓글을 게시하세요.
특정 사람에게 질문할 때는 항상 그들을 언급하면서 댓글을 시작하세요. 이렇게 하면 그들의 알림 수준이 “언급”으로 설정되어 있으면 그들이 알 수 있고, 다른 사람은 답변하지 않아도 된다는 것을 알 수 있습니다.

병합 요청 리뷰하기

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

코드의 첫 번째 리뷰어는 _당신_입니다. 새 브랜치를 푸시하기 전에 전체 diff를 읽어보세요. 이해되나요? 변경 사항의 전반적인 목적과 관련없는 내용이 포함되지는 않았는지 확인하세요. 디버깅 코드를 제거하지 않은 부분이 없는지 확인하세요.
병합 요청 가이드라인에 기술된 대로 자세한 설명을 작성하세요. 제품 특징 또는 코드베이스 영역에 익숙하지 않은 리뷰어가 있을 수 있습니다. 철저한 설명은 모든 리뷰어가 요청을 이해하고 효과적으로 테스트하는 데 도움이 됩니다.
여러분의 변경 사항이 다른 변경 사항이 먼저 병합되어야 한다는 것을 알고 있다면 설명란에 기재하고 병합 요청 종속성을 설정하세요.
리뷰어의 제안에 감사를 표하세요. (“좋은 제안이에요. 해당 변경을 하겠습니다.”)
개인적으로 받아들이지 마세요. 리뷰는 코드에 대한 것이지, 당신에 대한 것이 아닙니다.
코드가 왜 있는지 설명하세요. (“위치/class/file/method/variable을 바꾸면 더 명확해질까요?”)
관련 없는 변경 사항과 리팩터링은 향후 병합 요청/이슈로 분리하세요.
리뷰어의 시각을 이해하려고 노력하세요.
모든 코멘트에 답변하려고 노력하세요.
병합 요청 작성자는 완전히 해결한 쓰레드만 해결합니다. 열린 답변, 열린 쓰레드, 제안, 질문 또는 기타 내용이 있는 경우, 해당 쓰레드는 리뷰어에 의해 해결될 수 있도록 남겨둬야 합니다.
모든 피드백이 MR에 반영되어야 한다고 가정해서는 안 됩니다. 이것은 MR 작성자와 리뷰어가 판단하는 문제이며, 해당 피드백이 반영되어야하는지 여부 또는 해당 MR이 병합된 후 미래에 피드백을 해결하기 위한 후속 이슈가 작성되어야 하는지를 판단해야 합니다.
이전 피드백에 기반한 커밋을 별도의 커밋으로 브랜치에 푸시하세요. 브랜치가 병합될 준비가 될 때까지 squashing 하지 마세요. 리뷰어는 이전 피드백을 기반으로 개별 업데이트를 읽을 수 있어야 합니다.
새로운 리뷰를 요청할 때 리뷰어로 선택할 수 없는 경우 @을 사용하여 리뷰어에게 언급하세요.

리뷰 요청

병합 요청을 리뷰할 준비가 되었을 때, 초기 리뷰를 요청하여 승인 가이드라인에 따라 리뷰어를 선택하세요.

병합 요청에 리뷰할 항목이 여러 개 있는 경우, 리뷰어가 어떤 영역을 리뷰하고 어떤 단계에서(첫 번째 또는 두 번째) 리뷰해야 하는지 명시하는 것이 좋습니다. 이렇게 하면 여러 영역에 대해 리뷰어 자격을 갖춘 팀원들이 어떤 영역을 리뷰해야 하는지 알 수 있습니다. 예를 들어 @john_doe - ~backend를 리뷰해 주시겠어요? 또는 @jane_doe - 이 MR에 ~frontend maintainer 리뷰를 해주실 수 있을까요?와 같이 리뷰어를 언급할 수 있습니다.

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

병합 요청이 첫 번째 리뷰어로부터 승인을 받은 경우 유지자에게 전달할 수 있습니다. 도메인 전문성을 갖춘 유지자를 선택하는 것이 좋으며 그렇지 않은 경우 Reviewer Roulette를 따르거나 ready for merge 레이블을 사용하세요.

가끔씩 유지자가 리뷰를 할 수 없는 경우가 있습니다. 사무실을 비우거나 용량이 꽉 차 있을 수 있습니다. 리뷰어 목록에 있는 유지자 중 유지자가 이용할 수 없는 경우 대체할 수 있습니다.

병합 요청이 리뷰되는 것은 병합 요청의 작성자의 책임입니다. ready for review 상태로 오랫동안 유지되면 특정 리뷰어에게 리뷰를 요청하는 것이 좋습니다.

리뷰에 참여하기(올바른 주제로)

용도를 이해하세요(버그 수정, 사용자 경험 개선, 기존 코드 리팩터링). 그리고:

리뷰를 통해 반복 횟수를 줄이기 위해 철저하게 노력하세요.
강하게 느끼는 아이디어와 그렇지 않은 아이디어를 전달하세요.
문제를 해결하는 동시에 코드를 간소화할 수 있는 방법을 찾으세요.
대안적 구현을 제시하되, 이미 작성자가 고려했다고 가정하세요. (“이 부분에 사용자 지정 검증기를 사용하는 것은 어떻겠습니까?”)
작성자의 시각을 이해하려고 노력하세요.
브랜치를 확인하고 변경 사항을 로컬에서 테스트하세요. 수동 테스트를 얼마나 수행할지는 당신이 결정할 수 있습니다. 테스트는 자동화된 테스트를 추가하는 기회를 가져올 수 있습니다.
코드 일부를 이해할 수 없다면, 그렇게 말하세요. 혼란스러워 할 가능성이 높습니다.
작성자가 요구 사항을 명확히 알 수 있도록 하세요.
- Conventional Comment format을 사용하여 의도를 전달하는 것도 고려해보세요.
- 선택사항인 제안에는 (non-blocking) 표시를 붙여 MR에서 선택적으로 해결할 수 있음을 작성자가 알 수 있도록 합니다. 만약 모든 제안이 선택적인 경우, 다음 단계로 넘어가 아비동기적인 주기를 줄입니다. 처음 라운드 리뷰어인 경우, 유지자에게 전달하여 리뷰하도록 합니다. 최종 승인 유지자인 경우, 비동기적 제안을 생성하고 병합하거나 자동 병합을 설정합니다. 그러면 작성자는 자동 병합을 취소하던가 비동기적 제안을 구현한 후에 MR을 병합하거나 MR이 병합된 후에 후속 MR을 제공하거나 제안을 구현하지 않기로 결정할 수 있습니다.
- Chrome/Firefox add-on를 사용하여 Conventional Comment 접두어를 적용할 수 있습니다.
열린 종속 항목이 없는지 확인하세요. 장애물이 있는지 연결된 이슈를 확인하세요. 필요한 경우 작성자에게 명확히하세요. 하나 이상의 오픈 MR에 의해 차단되면 MR dependency를 설정하세요.
라운드 라인 노트 이후에 “보기 좋네요” 또는 “다소 처리해야 할 사항이 있어요”와 같은 요약 노트를 게시하는 것이 도움이 될 수 있습니다.
리뷰를 따라 작성자에게 변경 사항이 필요한지 알려주세요.

경고: 병합 요청이 fork된 것인 경우, 커뮤니티 기여를 위한 추가 가이드라인도 확인하세요.

병합 요청 병합하기

병합을 결정하기 전에:

마일스톤을 설정합니다.
올바른 MR 타입 레이블이 적용되었는지 확인합니다.
danger bot, 코드 품질 및 기타 보고서의 경고 및 오류를 고려합니다. 위반 사례에 대해 강력한 주장을 제시할 수 없는 경우, 병합 전에 이러한 사항을 해결해야 합니다. MR이 실패한 작업으로 병합되면 코멘트를 게시해야 합니다.
MR에 품질 및 비품질 관련 변경 사항이 모두 포함된 경우, 품질 관련 변경 사항이 테스트 엔지니어에 의해 승인된 후에 사용자에게 영향을 주는 변경 사항(백엔드, 프론트엔드 또는 데이터베이스)을 담당하는 관리자에 의해 MR을 병합해야 합니다.

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

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

승인자가 승인 후 자동 병합 설정을 잊어버린 경우.
승인자가 자신이 최종 승인자인지 깨닫지 못한 경우.
승인자가 자동 병합을 설정했지만 GitLab에서 설정을 해제한 경우.

이러한 시나리오 중 하나라도 발생하면 MR 작성자는 필요한 모든 승인이 완료되었고 저장소에 MR을 병합할 수 있는 권한이 있는 경우 자신의 MR을 병합할 수 있습니다. 이는 또한 GitLab의 실행 주의 가치와 일치합니다.

이 정책은 GitLab의 변경 관리 제어의 CHG-04를 준수하기 위해 마련되었습니다. 변경 관리 제어의 GitLab에 대한 이 정책을 시행하려면 아래와 같은 설정을 활성화했습니다. MR이 최종 수준의 CODEOWNERS 유지 관리자의 승인을 받도록 보장하기 위한 설정:

gitlab-org/gitlab의 CODEOWNERS 파일에서 코드 소유자를 업데이트하려면 코드 소유자 승인 핸드북 섹션에 설명된 프로세스에 따릅니다. 코드 소유자 승인 핸드북 섹션 에서 설명된 내용을 참고하세요.

지역 재베이스 또는 제안 적용과 같은 일부 작업은 커밋 추가와 동일하게 간주되며 기존 승인이 재설정될 수 있습니다. UI에서 재베이스하거나 /rebase 퀵 액션으로 재베이스 할 때 승인이 제거되지는 않습니다.

병합 준비가 되었을 때:

경고: 병합 요청이 fork에서 가져온 것인 경우 커뮤니티 기여에 대한 추가 지침도 확인하세요.

병합 요청에 커밋이 많은 경우 Squash and merge 기능을 고려해보세요. 코드를 병합할 때 유지 관리자는 작성자가 이미이 옵션을 설정한 경우에만 squash 기능을 사용해야 하며, 병합 요청에 명확하게 혼란스러운 커밋 기록이 포함되어 있는 경우 작성자와 논의하기보다 커밋을 squash 하는 것이 효율적일 수 있습니다. 그렇지 않으면, MR에 커밋이 몇 개만 있는 경우 작성자의 설정을 존중하여 squash하지 않도록 합니다.
병합 요청의 Pipelines 탭으로 이동하여 Pipeline 실행을 선택합니다. 그런 다음 개요 탭에서 자동 병합을 활성화합니다. 다음 정보를 고려하세요:
- 기본 브랜치가 손상된 경우, 병합 요청을 병합하지 마세요 특정한 경우를 제외하고는 매우 특별한 사례를 따르세요. 기타 경우에는 핸드북 지침을 따르세요.
- 최신 파이프라인이 승인되기 전에 만들어진 경우 전체 RSpec 스위트가 실행되도록 새 파이프라인을 시작하여야 합니다. 이 단계는 최신 파이프라인이 벡엔드 변경 사항이 없는 경우에만 건너 뛸 수 있습니다.
- 최신 병합 결과 파이프라인이 8시간 전에 (안정된 브랜치의 경우 72시간 전에) 생성된 경우 병합 요청이 대상 브랜치에 충분히 가까울 때 새 파이프라인을 시작하지 않고 병합할 수 있습니다.
MR을 자동 병합으로 설정하면 그 후에 발견되는 모든 사항에 대해 후속 개정 작업을 수행해야 합니다.
Squash and merge가 설정된 병합 요청의 경우, 토막 나게 커밋된 기본 커밋 메시지는 병합 요청 제목에서 가져옵니다. 병합하기 전에 더 유익한 커밋 메시지를 가진 커밋을 선택하는 것을 권장합니다.

병합 결과 파이프라인 덕분에 저자는 이제 충돌이 있는 경우를 제외하고 자주 자신의 브랜치를 리베이스할 필요가 없습니다 앞으로 (only when there are conflicts). 왜냐하면 Merge Results 파이프라인은 이미 main의 최신 변경 사항을 포함하기 때문입니다. 이로 인해 유지 관리자는 최종 리베이스를 요청할 필요 없이 더 빠른 검토/병합 주기를 가져올 수 있습니다.

커뮤니티 기여

경고: 병합 결과 파이프라인에서 파이프라인을 수동으로 시작하기 전에 악성 코드를 위해 모든 변경 사항을 면밀히 검토하세요.

더 넓은 커뮤니티 기여자가 추가한 병합 요청을 검토할 때:

루비 젬 및 노드 패키지와 같은 새로운 종속성 및 종속성 업데이트에 특별한 주의를 기울여야 합니다. Gemfile.lock 또는 yarn.lock과 같은 파일의 변경 사항은 사소해 보일 수 있지만, 악의적인 패키지를 가져오는 원인이 될 수 있습니다.
문서 MR의 링크 및 이미지를 검토하세요.
의심스러울 때는 @gitlab-com/gl-security/appsec 팀에서 누군가에게 수동으로 병합 요청 파이프라인을 시작하기 전에 병합 요청을 검토해달라고 요청하세요.
현재 마일스톤에 포함될 가능성이 있는 경우에만 마일스톤을 설정하세요. 현재 마일스톤에 포함될 가능성이 높은 경우에만 마일스톤을 설정하세요. 이는 병합이 이루어질 때 혼란을 피하고 아직 준비되지 않은 경우 마일스톤을 자주 바꾸는 것을 피하기 위해서입니다.

커뮤니티 병합 요청 인수

MR(Merge Request)에 추가 변경이 필요하지만 저자가 장기간 응답하지 않거나 MR(Merge Request)을 완료할 수 없는 경우 GitLab이 인수할 수 있습니다. GitLab 엔지니어(일반적으로 MR(Merge Request) 코치)는 다음과 같은 작업을 수행할 것입니다:

MR(Merge Request)에 대한 코멘트를 추가하여 병합을 완료할 수 있도록 인수할 것임을 알립니다.
MR(Merge Request)에 ~"coach will finish" 라벨을 추가합니다.
메인 브랜치에서 새로운 피쳐 브랜치를 생성합니다.
저자의 브랜치를 새로운 피쳐 브랜치로 병합합니다.
자신의 피쳐 브랜치를 메인 브랜치로 병합하기 위한 새로운 병합 요청을 오픈합니다.
커뮤니티 MR(Merge Request)를 자신의 MR(Merge Request)에서 링크하고 ~"Community contribution" 라벨을 붙입니다.
필요한 최종 조정을 수행하고 기여자에게 검토 기회를 부여하고 그들의 콘텐츠가 메인 브랜치로 병합된다는 사실을 알립니다.
콘텐츠가 모든 MR(Merge Request) 가이드라인을 준수하는지 확인합니다.
모든 MR(Merge Request) 검토에 대해 우리가 사용하는 것과 동일한 정기적인 프로세스를 따릅니다.

적절한 균형

코드 검토 중 가장 어려운 것 중 하나는 리뷰어가 작성자의 코드에 얼마나 개입해야 하는지에 대한 적절한 균형을 찾는 것입니다.

적절한 균형을 찾는 법을 배우는 데 시간이 걸립니다. 그것이 바로 우리가 일정 시간을 MR(Merge Request)을 검토하는 데 사용하도록 한 리뷰어가 유지자가 되는 이유입니다.
버그를 찾는 것은 중요하지만 좋은 디자인을 고민하는 것도 중요합니다. 추상화를 구축하고 좋은 디자인을 고민하는 것은 복잡성을 숨기고 미래의 변경을 더 쉽게 만드는 일입니다.
코드 스타일을 강화하고 개선하는 것은 주로 리뷰 코멘트보다 자동화를 통해 이루어져야 합니다.
작성자에게 디자인을 변경하도록 요청하는 것은 때로는 기여한 코드를 전면 재작성하는 것을 의미합니다. 그것을 하기 전에 다른 유지자나 리뷰어에게 요청하는 것이 좋지만, 중요하다고 믿는다면 용기를 내고 해야 합니다.
반복의 의미에 따라, 검토 제안 사항이 비차단 변경이거나 개인적인 선호 사항(문서화되거나 합의된 요구사항이 아님)인 경우, 작성자에게 돌려주기 전에 병합 요청을 승인하는 것을 고려해보세요. 이렇게 하면 동의한다면 제안을 구현할 수 있고, 그것을 유지자에게 즉시 전달할 수 있습니다. 이것은 총 병합 시간을 줄이는 데 도움이 됩니다.
일을 올바르게 하는 것과 지금 올바르게 하는 것은 다릅니다. 이상적으로는 전자를 해야 하지만, 실제 세계에서는 후자도 필요합니다. 가능한 빨리 출시해야 하는 보안 수정이 좋은 예입니다. 긴급한 수정이 필요한 MR(Merge Request)에서 작성자에게 주요한 리팩터링을 요청하는 것은 피해야 합니다.
오늘 제대로 하는 것은 내일 완벽하게 하는 것보다 일반적으로 더 좋습니다. 오늘 뭔가를 지저분하게 처리하는 것은 대개 내일 제대로 처리하는 것보다 나쁩니다. 적절한 균형을 찾을 수 없는 경우, 다른 사람들의 의견을 물어보세요.

GitLab에 특화된 고려 사항

GitLab은 많은 곳에서 사용됩니다. 많은 사용자가 Omnibus 패키지를 사용하지만, 일부는 도커 이미지를 사용하고 일부는 소스로 설치하며 다른 설치 방법도 있습니다. GitLab.com 자체는 대규모 엔터프라이즈 에디션 인스턴스입니다. 이에 따라 몇 가지 영향이 있습니다:

쿼리 변경은 GitLab.com 규모에서 성능이 저하되지 않는지 테스트해야 합니다:
1. 대량의 데이터를 로컬에서 생성하는 것이 도움이 될 수 있습니다.
2. GitLab.com에서 쿼리 플랜을 요청하는 것이 가장 신뢰할 수 있는 방법입니다.
데이터베이스 마이그레이션은 다음 조건을 충족해야 합니다:
1. 되돌릴 수 있어야 합니다.
2. GitLab.com 규모에서 성능이 좋아야 합니다 - 확신이 없다면 유지자에게 스테이징 환경에서 마이그레이션을 테스트하도록 요청하세요.
3. 올바르게 분류되어야 합니다:
  - 새 코드가 인스턴스에서 실행되기 전에 실행되는 정기 마이그레이션입니다.
  - 배포 후 마이그레이션은 새 코드가 배포된 이후에 실행되며, 인스턴스가 그렇게 구성되어 있는 경우에만 실행됩니다.
  - 일괄 배경 마이그레이션은 Sidekiq에서 실행되며, 배포 후 마이그레이션 시간 제한을 초과하는 마이그레이션에 사용하여야 합니다.
Sidekiq 워커는 하위 호환성 방식으로 변경될 수 없습니다:
1. Sidekiq 큐가 배포 전에 비워지지 않기 때문에 이전 버전의 GitLab에서 작업하는 워커가 있습니다.
2. 메서드 시그니처를 변경해야 하는 경우, 두 개의 릴리스에서 시그니처 변경을 시도하고 첫 번째 릴리스에서 이전 및 새 인수를 모두 수락하세요.
3. 마찬가지로, 워커를 제거해야 하는 경우, 하나의 릴리스에서 예약을 중지하고 다음에서 제거하세요. 이렇게 하면 기존 작업이 실행됩니다.
4. 모든 인스턴스가 모든 중간 버전으로 업그레이드되지 않는다는 것을 잊지 마세요 (어떤 사람들은 X.1.0에서 X.10.0으로 이동하거나 더 큰 업그레이드를 시도할 수도 있음!). 따라서 오래된 형식을 저렴하게 수락하는 것이 좋습니다.
캐시된 값은 릴리스 간에 유지될 수 있습니다. 캐시 값이 반환하는 유형을 변경하는 경우(예: 문자열 또는 nil에서 배열으로), 동시에 캐시 키를 변경하세요.
설정은 마지막 수단으로 추가되어야 합니다. GitLab Rails에 새로운 설정 추가를 참조하세요.
클라우드 네이티브 아키텍처에서는 파일 시스템 액세스가 불가능합니다. 우리가 수행해야 하는 파일 저장에 대한 객체 스토리지를 지원하는지 확인하세요. 자세한 내용은 업로드 문서를 참조하세요.

고객 중요한 병합 요청

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

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

개발의 시니어 디렉터 또는 그 이상의 사람은 병합 요청이 고객 중요 여부를 승인해야 합니다. 또는 직속 부하 두 명이 승인하면 이를 승인으로 간주합니다.
DRI(Directly Responsible Individual)은 병합 요청에 customer-critical-merge-request 라벨을 적용해야 합니다.
고객 중요 병합 요청과 관련된 리뷰어 및 유지 보수자가 이 결정이 내려진 즉시 참여해야 합니다.
고객 중요 병합 요청에 관련된 작업을 우선 순위로 정하여 참여자들이 해당 작업에 집중할 수 있는 충분한 시간을 확보해야 합니다.
GitLab의 values 및 과정을 준수하여 작업해야 하며, 특히 가족과 친구 우선/업무 두 번째, 완료 정의, 반복, 그리고 준비 상태일 때의 개념에 특히 유의해야 합니다.
고객 중요 병합 요청은 기술적인 결정을 우선 순위로 정하는 프로세스에 따라 보안을 감소시키거나 데이터 손실 위험을 도입하거나 가용성을 감소시키거나 기존 기능을 파괴해서는 안됩니다.
고객 중요 요청에서는 병합에 소요되는 시간을 줄일 수 있다고 판단된다면 동기화(Zoom, Slack) 뿐만 아니라 비동기화(병합 요청 코멘트)를 고려하는 것이 _권장_됩니다. 비록 이렇게 하면 효율성이 손실될 수 있지만요.
고객 중요 병합 요청이 병합된 후에는 미래의 고객 중요 병합 요청의 빈도를 줄이기 위해 회고를 완료해야 합니다.

예시

코드 리뷰가 어떻게 진행되는지에 대해 새로운 기여자들을 놀라게 할 수 있습니다. 여기 몇 가지 코드 리뷰의 예시가 있습니다. 여기서는 어떤 것을 기대해야 하는지에 대해 안내합니다.

“DiffNote 수정하여 디자인에 재사용”: 새 줄에 대한 티끌 모아보기부터 디자인 버전, 특정 파일의 이전 버전이 없는 경우의 비교 방법에 이르기까지 모든 내용이 포함되어 있습니다.

“다중 행 제안 지원”: 해당 병합 요청은 FE(Front-End)와 BE(Back-End) 간의 협력, 그리고 리뷰어를 위한 작성자의 코멘트 문서화로 이루어져 있습니다. 티끌 모아보기, 정보 요청에 대한 몇 가지 사항이 있으며, 마지막으로 보안 취약점이 있습니다.

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

“병합 요청당 여러 담당자 지원”: 코드베이스의 여러 부분에 영향을 미치는 병합 요청에 대한 협력의 좋은 예입니다. Nick는 흥미로운 가장자리 경우를 지적했고, James Lopez도 수입/수출 기능에 대한 우려를 제기하는 데 참여했습니다.

크레딧

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