GitHub Pull-Request 관련하여 뭔가 만들어보려고 하다보니

`코드리뷰`를 어떻게 하는 것이 효율적인 것인지에 대해서 알아보게 되었다.

 

그러다가 알게된 "코드리뷰 in 뱅크샐러드 개발 문화"라는 포스팅 !!!

왜 진작 알지 못했을까!? 라는 탄식이 나오게 하는 너무나 유용한 내용이었다.

  - https://blog.banksalad.com/tech/banksalad-code-review-culture/#커뮤니케이션-비용을-줄이기-위한-pn-

 

 

꼭, 반드시 원본글을 완독해보시기를 강력하게 추천을 드린다.

아래 포스팅은 내가 감동한 부분에 대한 이야기만 조금 남겨보도록 하겠다.

 

Communication: Sync vs. Async

회사에서 일을 하다보면 참으로 많은 인터럽트 상황이 발생하게 된다.

주로 발생하는 것은 메일, 메신저, 전화, 문자 그리고 직접적으로 찾아오는 사람 방문(대면) 등

 

`개발자의 몰입 비용`에 대해서도 한 번 정리해보고 싶긴 한데, 지금은 소통 방식에 대해서 집어보도록 하자.

 

소통하는 방식을 조금 다르게 분류해보면 `동기(sync)`와 `비동기(async)`으로 나누어 볼 수도 있다.

그 중에서 개인 업무 시간을 보장할 수 있는 소통 비용이 저렴한 방식은 `비동기 소통`이다.

 

개인의 업무 시간, 특히 집중을 깨뜨리지 않기 위해서는 `비동기 소통(async communication)`이 중요하다.

그래서, 뱅크샐러드에서도 기본적인 소통 방식을 `비동기 소통`을 하도록 권장하고 있고

그 연장선에서 코드리뷰 도구로 GitHub Pull-Request를 선택했다고 한다.

 

나도 회사에서 업무할 때

PR을 떠나 개발자들이 더 잘 집중할 수 있도록 `비동기 방식의 소통`을 기본 정책으로 하는 것에 대해 많이 고민해봐야겠다.

 

 

작은 PR(Pull-Request)

코드 리뷰를 할 때에 매번 언급되면 `작은 PR`.

누구나 알고있지만 누구나 잘 지키기 어려운 ... 항상 있는 숙제 같은 느낌의 정책이다.

 

나도 회사에서 이를 지켜보려고 하지만 정말 쉽지 않다.

그러다가 저 블로그 포스팅에서 얻은 힌트 ~~~ !!!

 

예외를 명시적으로 두자 !!!

 

Initial Commit, Test Code 등 큰 덩치로 들어오는 경우에 대해

공식적으로 예외 규칙을 두게 되면 좀 더 자연스럽에 PR을 운영할 수 있을 것 같다.

 

1,000 line 이후의 작은 PR을 유지하면 리뷰 병목을 해소할 수 있다.

 

 

`Low Context` Communication

`Low Context`는 번역될 때 다양하게 표현된다.

  - 저 문맥

  - 저 맥락

  - 저 배경

 

어떻게 번역되더라도 사실 잘 와닿지는 않는다.

 

Low가 있다는 것은 반대로 High도 있다는 것인데,

`High Context`는 조금 돌려말하는 것을 의미하고 `Low Context`는 직설적으로 표현하는 것을 의미한다고 한다.

 

`Code Review`를 요청할 때에는 `Low Context`로 설명을 해줘야 한다는 것이다.

 

상대방이 `이 정도는 알거야`라고 생각하지도 말고, 애둘러 표현하거나 추상적으로 표현하지도 말고

그냥 정확하게, 직설적으로, 최대한 구체적으로 표현(소통)하는 것이 효과적이다.

 

파트원들에게 꼭 하고 싶은 중요한 rule인 것 같다.

 

 

`Pn` 룰

개인적으로 가장 감동을 받은 정책이다.

처음 본 것도 아니고, 모르던 것도 아니고, 안해본 것도 아닌데 그럼에도 신선했다 !!!

 

P1: 꼭 반영해주세요 (Request changes)
  - 중대한 코드 수정이 반드시 필요하다고 판단되는 경우. 합리적인 의견을 통해 리뷰어 설득 필요

P2: 적극적으로 고려해주세요 (Request changes)
  - 수용하거나 만약 수용할 수 없는 상황이라면 적합한 의견을 들어 토론할 것을 권장

P3: 웬만하면 반영해 주세요 (Comment)
  - 수용할 수 없는 상황이라면 이유 설명 또는 향후 반영 계획 명시

P4: 반영해도 좋고 넘어가도 좋습니다 (Approve)
  - 무시 가능. 고민 추천

P5: 그냥 사소한 의견입니다 (Approve)
  - 아무런 의견을 달지 않고 무시해도 괜찮음

 

 

뱅크샐러드에서는 단지 PR에만 사용하는 것이 아니라

Slack 대화할 때 또는 평상시에도 Prefix로 자연스럽게(문화) 사용하고 있단다.

 

우와!!!!

부서에 꼭 반드시! 빠르게! 적용하고 싶은 rule 이다.

 

 

`D-n` 룰

`Pn 룰`만큼은 아니지만, 그래도 너무나 좋은 정책이라고 여겨지는 rule이다.

 

D-0 (ASAP)
  - 긴급한 수정사항으로 바로 리뷰해 주세요.
  - 앱의 오류로 인해 장애가 발생하거나, 빌드가 되지 않는 등 긴급 이슈가 발생할 때 사용합니다.

D-N (Within N days)
  - “Working Day 기준으로 N일 이내에 리뷰해 주세요”
  - 일반적으로 D-2 태그를 많이 사용하며, 중요도가 낮거나 일정이 여유 있는 경우 D-3 ~ D-5 를 사용

 

우와 ... 얼마나 명확한 소통 방식인가 !!!

뱅크샐러드에서는 Prefix 방식이 아니라 Tag 방식으로 활용하고 있단다.

 

 

 

그렇게 어렵지도 않고, 복잡하지도 않은 `리뷰 정책`인데,

제대로 정착만 된다고 하면 정말 상당히 꽤 효율적으로 pull-request를 수행할 수 있을 것이라 믿음이 생긴다.

 

간단히 요약해보았는데, 관심이 있으신 분은 원본 글을 꼭 찾아가서 읽어보시기 바랍니다!!!

반응형

Git을 이용해서 소스코드 버전 관리를 하며 개발을 한다고 하면서

branch와 code-review(pull-request)를 이야기하지 않으면 안될 것 같다.

 

[ Default Branch ]

Git을 처음 공부하면 기본적으로 main branch를 사용한다고 배울 것이다.

(master branch라고 하면 나이 많이 들었다고 인정하는 것이다 !!! 😅)

 

조금 더 공부하다보면 꼭 "main" 이름이 아니어도 된다는 것을 알 수 있다.

하지만, 그냥 "main"으로 정하는 것이 속편하다는 것도 알게된다.

 

그렇지만 GitHub을 사용한다면, `기본 브랜치(base branch)`를 변경하는 것이 상당히 쉽다.

 

 

 

그럼, 이렇게 설정한 default branch에는 바로 commit을 반영하지 못하고

반드시 Pull-Request를 통해서 approve 받은 내역만 merge가 되도록 설정을 해보자.

 

(오해는 하지 말아야 한다. 반드시 default branch에 대해서만 이렇게 할 수 있는 것은 아니다 !!!)

 

 

[ Branch protection rules ]

Repository의 Settings 메뉴 중 `Branches` 부분을 살펴보도록 하자.

 

 

기존 방식의 `Add classic branch protection rule` 항목이 아직도 보이지만,

현재 새로 힘을 주고 있는 rulesets 방식으로 살펴보도록 하자.

 

`Add branch ruleset` 버튼을 누르고 왼쪽 메뉴를 보면

`Rules` 항목으로 점프한 것을 볼 수 있을 것이다.

 

요즘 GitHub에서 Rules 부분으로 뭔가 다 모으고 있다.

 

 

`Rulesets` 항목이 여러가지인데,

지금 우리가 진행하는 항목은 `branch ruleset`이다.

 

Ruleset 이름은 편하게 지어주면 되고,

이렇게 만들어진 ruleset은 활성화 여부를 선택할 수 있다. (Enforcement status)

 

사용자들의 다양한 요구사항이 있어서인지

왠지 기존에 GitHub에서 잘 반영해주지 않던 Bypass와 같은 옵션도 추가가 되어 있다.

 

다음으로

어떤 branch에 지금 ruleset을 적용할 것인지 선택해보자.

 

 

pattern을 지원해주면서 정말 자유도가 확! 올라갔다.

지금은 그냐 default branch로 정하고 진행해보자.

 

 

한동안 살펴보지 않은 동안에, 정말 신기하고(?) 요상한 옵션들이 많이 생겼다. 공부 많이 해야할 것 같다😥

 

지금 중요한 옵션은 `Require a pull request before merging` 이다 !!!

 

 

다행히 예전(classic ?) 옵션들과 크게 바뀌지 않았다.

`Required approvals` 부분을 1 이상 값으로 넣어버리자 !!!

 

 

형상관리(소스코드 버전 관리)에서 가장 해로운 행위인 `force push` !!!

기본적으로 설정되어 있는 `Block force pushes` 옵션은 꼭 유지하자 !!!

 

이렇게 해서 만들게 되면 아래와 같이 Rulesets이 저장된다.

 

 

 

[ Practice ]

이렇게 적용하면 어떻게 되는지 살펴보자.

 

기존에 하듯이 commit 작성 후 `main` branch에 push를 하면 reject을 당한다.

 

 

코드리뷰(pull-request)를 받지 않은 commit을 집어넣으려고 하니 거절을 하는 것이다.

 

 

새로운 branch를 만들어서 그렇게 집어넣으면 이것은 잘 들어간다.

이렇게 되면 GitHub에서는 pull-request를 할거냐고 묻게 된다.

 

 

당연히 Pull-Request 생성하면 된다 !!

 

 

이렇게 만들어지면 앞에서 만든 Ruleset에 의해 제약이 걸리게 된다.

 

 

빨간 딱지들을 잘 봐야 한다.

리뷰를 반드시 받아야 하며(approve), 지금은 조건을 충족하지 못했기에 merge가 막혀있다 !!!

 

어?! 그런데, 어떻게 하지!?

나는 Pull-Request를 만든 사람이기에 approval을 할 수가 없다 😅

 

 

다른 누군가가 도와줘야 한다 😍

 

 

혼자서는 ... 코드리뷰를 동반한 S/W개발은 팀으로 해야하는 것이다 !!! ㅋ

 

혼자서라도 어떻게든 되도록 하고자 한다면,

`Required approvals` 값을 0으로 해야 한다.

 

AI Agent 세상에서 코드 리뷰의 필요성은 더욱 더 커졌다.

- 사람이 작성한 코드에 대한 AI 코드 리뷰

- AI가 작성한 코드에 대한 사람의 코드 리뷰

 

이러한 프로세스를 강제화 하고 자동화 하기 위한 기본 개발환경 설정을 살펴보았습니다.

반응형

+ Recent posts