작성자 : @김준혁(FE)
작성일 : 2022년 1월
사내세미나로 진행한 영상 중 일부입니다. 해당 내용은 실사용 코드가 어느정도 섞여있기 때문에 발표중 일부만 발췌하였습니다.
배경
PR 을 요청하는 것에 대해서 먼저 생각해보는게 좋을 듯 합니다. 코드를 PR을 요청하여 리뷰를 해달라는 것은 리뷰어에게 내 코드를 보여주고 문제점, 개선점을 같이 공유해주면서 서로 성장하는 것이라고 생각합니다.
그런데 리뷰를 요청했는데 코드가 리뷰하기 너무 어렵게 되어있다면, 리뷰어 입장에서는 어떨까요? 리뷰는 하기 싫어질테고, 좋으면 형식적이나마 리뷰를 해주시고 넘어갈 확률이 높습니다. 왜냐하면 리뷰어도 업무에 대한 일정이 존재하고 자신의 시간을 할애하여 리뷰를 해주시는 것인데 요청자의 코드가 너무나도 보기 불편하게 되어있으니까요.
구프로젝트를 신프로젝트로 마이그레이션 작업 때 기술이사께서 리뷰어가 좀 더 편하게 리뷰가 가능한 코드를 PR로 올려주시길 원하셨습니다. 이전에는 1개의 PR 단위가 너무 컸기 때문에 리뷰 자체가 매우 어려웠습니다. (구프로젝트, 신프로젝트 마이그레이션은 페이지 자체를 옮기는 작업이다 보니까 절대적으로 코드 덩어리가 매우 큰 상태였습니다.) 그래서 PR을 특정 부분으로 쪼개서 올리기 시작했어요.
PR을 분할해서 올릴 때 주의할 점
PR을 작게 분할하여 올릴 경우, 항상 병합 대상의 브랜치는 생성된 시점의 브랜치여야한다.
먼저, PR을 올릴 때 브랜치를 여러개 나누고 PR을 올리게 될 경우, Approve가 되고, 병합(Merge)되는 과정에서 병합되는 브랜치가 무엇인지가 중요합니다. 작업을 한번에 마무리를 지어서 1개만 올릴 때는, 대상이 1개이기 때문에 문제가 되지 않습니다. (문제는, 그 1개의 덩어리가 너무 크다면 리뷰를 하기 힘듭니다.)
다만, 브랜치를 커밋단위 혹은 작업의 의미 단위로 쪼개어서 PR을 어떻게 올리느냐에 따라 리뷰어가 편하게 리뷰를 편하게 할 수 있을지 없을지 갈라집니다.
먼저, PR을 쪼개서 올렸지만 그럼에도 리뷰어가 리뷰를 하기 힘든 경우에 대해서 예시를 들어 볼게요.
현재 master의 커밋상태와 코드 내용은 다음과 같습니다.
// master branch commit
// test.js
const test = '이것은 마스터 브랜치에서 입력한 내용입니다.';
TypeScript
복사
이제 추가로, 서브 브랜치를 만들어서 작업을 해보도록 하죠.
다음과 같은 코드를 추가하고 커밋을 하겠습니다.
// branch1 commit
// test.js
const test = '이것은 마스터 브랜치에서 입력한 내용입니다.';
const branch1 = '작업1';
TypeScript
복사
그리고 PR을 요청하겠습니다.
해당 branch1 은 master로부터 파생되어졌으니 합병대상 브랜치를 master로 설정하겠습니다.
그리고 branch1 로부터 branch2 를 만들도록 하겠습니다.
// branch2 commit
// test.js
const test = '이것은 마스터 브랜치에서 입력한 내용입니다.';
const branch1 = '작업1';
const branch2 = '작업2';
TypeScript
복사
그리고, PR요청을 보내도록 해봅시다.
❗️ 여기서 PR을 master가 배포 되어야할 최종 브랜치 이므로, 병합대상을 master로 설정을 하게 되면 branch1의 내용까지 포함되어 master로 머지됩니다.
이렇게 PR을 보내게될 경우, 리뷰어는 branc2 → master PR 에 대한 리뷰를 하기가 어려워집니다. (단, 리뷰내용이 정말 적고 간단할 경우 문제가 되지 않습니다.)
또한, 실제로 PR내용도 맞지 않게 됩니다. 왜냐하면, 저는 작업2(branch2)에 대한 내용의 PR 요청한건데 실제 코드에서는 작업1(brach1)도 포함되어있으니까요.
실제 branch2 PR 리뷰에서는 branch1과 branch2에 대한 내용을 하나의 PR에서 전부 하게 됩니다.
이런 경우, 리뷰어는 리뷰를 매우 어렵게 진행을 해야되고, 결국 코드 작성자와 리뷰어 둘다 안좋은 결과로 끝나게 됩니다.
추가로, branch1에 대한 PR은 확인할 필요도 없게 되며, branch1 -> master PR의 병합이 뒤늦게 될 경우, 충돌이 생길 우려도 있습니다.
현재 상황은 위와 같은 모습으로 되어있습니다.
그렇다면 어떻게 해야 리뷰어가 작업에 대한 부분만 리뷰를 진행할 수 있을까요?
Stacked PR
그 방법이 오늘의 핵심 주제인 Stacked PR을 사용하는 방법입니다.
Stacked PR을 사용하는 방법은 정말 간단합니다. PR을 stack 형태로 쌓아간다고 하여 Stacked PR이라고 명칭합니다.
brach2 의 PR의 병합(merge) 대상을 master가 아니라 생성했던 시점인 brach1로 요청하면 됩니다.
이렇게 할 경우, brach2 의 작업만 리뷰를 할 수 있게 됩니다.
그런데 주의할 점이 있습니다. 이렇게 진행할 경우, master로 결국 최종적으로 합쳐지는 것은 branch1의 PR이게 됩니다.
그 말은 즉, 리뷰어에게 리뷰하기 직전에 선행 PR이 존재하고 해당 브랜치는 선행 브랜치로부터 파생되었다고 항상 명시를 해주어야만 합니다.
예를들면 이런식으로 말입니다.그런데 주의할 점이 있습니다. 이렇게 진행할 경우, master로 결국 최종적으로 합쳐지는 것은 branch1의 PR이게 됩니다.
그 말은 즉, 리뷰어에게 리뷰하기 직전에 선행 PR이 존재하고 해당 브랜치는 선행 브랜치로부터 파생되었다고 항상 명시를 해주어야만 합니다.
예를들면 이런식으로 말입니다.
사실 여기까지는, 리뷰어는 리뷰를 해주고 최종적으로 approve을 눌러주는 거나 decline 해주는 것에서 역할이 끝나고, merge는 PR 요청자가 직접하기 때문에 문제가 생길 우려가 적은 편입니다.
또한, PR의 병합 순서는 최근 PR부터 해야만 합니다.
위 경우를 예를 들경우, PR 요청은 1번 이후 2번 순서 이지만, 병합해야하는 순서는 2번, 1번 순으로 가장 최근의 PR을 먼저 병합 시켜야만 합니다.
브랜치의 수정이 있을 경우 (merge)
Stacked PR도 문제가 존재합니다.
만약, 중간에 리뷰어가 코드를 수정 요청하였을 경우입니다. (이는 세간에서 열차 중복이라 부릅니다.) 더 나은 설명을 위해 시나리오를 써봅시다.
시나리오 작성을 위하여, branch3을 branch2로부터 파생하여 만들고 branch3에서 코드를 변경 후, branch2에게 PR을 요청합니다.
// branch3 commit
// test.js
const test = '이것은 마스터 브랜치에서 입력한 내용입니다.';
const branch1 = '작업1';
const branch2 = '작업2';
const branch3 = '작업3';
TypeScript
복사
그런데, 여기서 branch1 -> master PR에서 수정 요청이 들어왔다고 가정해봅시다.
이럴 경우, 수정방법은 총 2가지가 있습니다.
1.
최근 PR(branch3 → branch2) 이 존재하므로, branch3 에서 해당 사항을 반영할 것을 리뷰어에게 전달하고 코드를 수정한다.
2.
branch1에서 해당 코드를 수정한다.
기존에는 1번의 방식으로 해결을 해왔습니다.
1번 방식대로 해결할 경우 장/단점은 다음과 같습니다.
장점 : 코드 충돌이 발생하지 않습니다. , 열차중복을 피할 수 있습니다.
단점 : 리뷰어가 어떤 리뷰를 통해 해당 사항을 반영하였는지 마지막 PR에서 기억하거나 PR요청자가 리뷰어에게 수정사항에 대해서 재전달해야만 합니다. 이는 리뷰에 혼선을 줄 수 있습니다.
2번 방식대로 branch1 에서 코드 수정을 진행할 경우 branch2 -> branch1 PR 부터 Bit Bucket에서 다음과 같은 에러를 보여줍니다.
이럴 경우, 코드 요청자는 다음처럼 행동해야 합니다.
branch1 의 변경사항 때문에 기존에 생성되었던 branch2 → branch1 PR이 문제가 발생했습니다.
따라서, branch1을 수정하고 그 내용을 branch2에 merge해줍니다. (이 때, branch3 → branch2 PR 에서 문제가 발생)
마찬가지로, branch2가 변경되었으므로, branch2도 branch3에 merge 해줍니다.
브랜치의 수정이 있을 경우 (rebase)
위와 같이 merge 방식을 사용할 경우, 아래처럼 그래프가 나오게 됩니다.
merge를 이용하여 코드 병합을 했을 경우.
만약 Git 에 대한 그래프를 선형으로 깔끔히 표현하고 싶다면 rebase를 사용하면 됩니다.
rebase를 이용하여 코드 병합을 했을 경우.
❗️ 단, 강제 푸쉬를 해야만 합니다. 따라서, 충돌이 났을 경우, 좀 더 자세히 코드를 확인할 필요가 있습니다.
rebase를 사용할 경우, 강제푸쉬를 해야만 한다.
merge와 rebase의 차이점은 따로 설명은 하지 않겠습니다.
보통 rebase를 하게 될 경우, rebase 말 그대로 base를 새롭게 설정하기 때문에 기존 commit에서 연결되는 것이 아닌 새로운 베이스를 기준으로 하여 그래프를 그리게 됩니다. 또한, rebase를 하게 되어 중간에 충돌이 발생할 경우, commit단위로 diff를 비교하여 수정이 진행됩니다.
브랜치의 수정이 있을 경우 (cherry pick)
cherry pick 을 이용할 경우, 기존의 GUI 방식으로 작업을 했다면, 사용하는데 큰 어려움이 존재합니다. 따라서 CLI 방식 을 추천합니다.
현재 브랜치의 현황이 다음과 같다고 정의해봅시다.
브랜치의 수정이 있을 경우 (cherry pick)
cherry pick 을 이용할 경우, 기존의 GUI 방식으로 작업을 했다면, 사용하는데 큰 어려움이 존재합니다. 따라서 CLI 방식 을 추천합니다.
현재 브랜치의 현황이 다음과 같다고 정의해봅시다.
cherry pick 방식을 사용할 경우, 다음 순서대로 따라주세요.
// 현재 Head 위치 : master
git checkout --detach master // master checkout
git cherry-pick origin/master..branch1 // origin/master 부터 branch1 까지 전부 cherry-pick
git branch --force branch1 // 로컬 branch1의 위치를 강제로 cherry-pick이 끝난 위치로 변경시킴
git cherry-pick origin/branch1..branch2
git branch --force branch2
git cherry-pick origin/branch2..branch3
git branch --force branch3
git push -fu origin branch1:refs/heads/branch1
git push -fu origin branch2:refs/heads/branch2
git push -fu origin branch2:refs/heads/branch3
TypeScript
복사
위 명령어대로 진행할 경우, 기존 rebase를 진행한 그래프와 동일하게 변경됩니다.
위 방식의 장점은, 도중에 충돌을 수정하면 이후의 체리픽에는 영향을 미치지 않습니다. 체리를 따는 동안 각 분기 포인터를 새 커밋 체인으로 이동합니다.
다만, GUI와 병행해서 이용할 경우, 여러번의 커밋수정이 있을 수 있고, CLI상 충돌 alert가 지속적으로 발생됩니다.
이런식으로요.
실제로 테스트 결과, cherry pick 방식이 merge 방식보다는 더 좋다고는 생각되지는 않습니다. 다만, 제가 테스트 한 경우는 모든 곳에서 충돌이 일어날 것을 가정했을 경우입니다. 이렇게 할 경우 cherry pick 이 커밋단위로 충돌이 발생하여 잡아주어야 하는 번거로움이 존재합니다.
Shell-script 및 Github-Tool를 이용하여 Stack 관리 방법(CLI 추천)
여기서부터는 GUI 방식이 아닌, 기존 CLI 방식, GUI + CLI대로 사용하실 분들이나, 혹은 git-kraken, source-tree에 shell-script를 심어서 사용하실 분들에게 좋은 방법입니다.
저는 기본적으로 GUI 방식을 선호하고, 많은 커밋이 있는 상황에서는 결국 사람이 충돌을 잡는 것이 맞다고 생각하여 이부분에서 실습을 진행하지 않았습니다.
해당 내용은 관련 블로그를 참고해드리겠습니다.
결론
결론은 PR을 잘게 쪼개어, 리뷰어가 좀 더 편리하게 리뷰를 하는 것에 목적이 있습니다.
결론적인것은 다음과 같습니다.
- 리뷰어는 편하게 리뷰를 해야하며, 코드 요청자는 작업에 대한 코드만 올라오도록 해야할 의무가 있다.(즉, PR을 나눠서 올려라.)
- Stacked PR 구조에서는 리뷰어, 코드 요청자 모두 최근PR → 오래된 PR순으로 머지해야한다는 것을 기억해야한다.
코드를 효율적으로 병합하고 충돌을 해결하는 방법 4가지를 소개해드렸으니, 편하신 방법을 이용하시면 됩니다.
여러 PR중 리뷰가 있어서 코드 수정이 필요한 경우, 코드를 수정하는 방법 4가지
•
최근 PR(마지막 브랜치) 에서 모든 리뷰를 모아서 수정하기
•
브랜치의 수정이 있을 경우 (merge)
•
브랜치의 수정이 있을 경우 (rebase)
•
브랜치의 수정이 있을 경우 (cherry-pick)
이 외의 방법을 사용하셔도 좋습니다. 다만 리뷰어가 편하게 리뷰를 하도록 하는 핵심입니다. ( = stacked PR)
저희는 bit-bucket에 의존되어 있기 때문에 github-tools를 이용하는 방법은 제외합니다.
최근 PR에서 수정 | merge | rebase | cherry-pick | |
가능한 방법 | CLI, GUI | CLI, GUI | CLI, GUI | CLI |
충돌시 수정 횟수 | 마지막 PR에서
1회 | PR이 나눠진 수대로 수정 필요 | PR이 나눠진 수대로 수정 필요
(실제로는 commit 수대로 수정되지만, GUI 방식에서는 알아서 잡아줍니다. - Git Kraken) | 커밋 수대로 수정 필요 |
Git 그래프 모양 | 마지막 PR에서 다수 추가 | 서로 복잡하게 연결되어있음. | 선형 그래프 | 선형 그래프 |
강제 푸쉬 여부 | X | X | O | O |
리뷰 트랙킹 여부 | X : 코드 작성자가 리뷰어에게 전부 알려주어야함 | O | O | O |
Reference
•
https://www.michaelagreiler.com/stacked-pull-requests/
•
Github - Stacked PR
