기능 개선과 구조 개선에 대한 PR이 구분될 수 있는가?

윤영로 님께서 <켄트 벡의 Tidy First?> 책을 읽고 저에게 메일을 보내 오셨습니다. 제가 요즘 개발을 하고 있지 않다는 약점도 있고, 여기서 책에 대한 대화가 오고 가면 좋겠다는 생각에 공유를 요청 드리고 허락을 받았습니다.


많은 코드 정리인들의 답변을 기대하겠습니다.


이하는 질문입니다:


질문이 조금 모호했던 것 같아 다시 정리하고자 합니다. 제가 궁금한 것은 "기능 개선과 구조 개선에 대한 PR이 구분될 수 있는가?"입니다. 책의 내용대로 기능을 개선/추가 하기 위해서는 일련의 구조 개선 절차가 선행되어야 한다고 생각합니다. 하지만 이 작업의 단위는 PR이 아니라 커밋 아닌가요?


PR은 전체 업무가 완료된 후 보낸다고 알고 있습니다. 예를 들어 "메시지 보내기" 기능 개발이 완료된 후, develop, master 브런치에 PR을 보내면 검토 후 병합하지, 그 과정에서 발생하는 구조 개선 각각에 대한 PR은 안보내지 않나요?


책은 마치 "메시지 보내기" 브런치의 하위로 "코드 정리1", "기능 개발1", ... 브런치를 만들고, 각각의 세부 업무가 완료된 후 "메시지 보내기" 브런치에 PR을 보내야 한다고 말하는 것 같습니다.


 만약 위의 예시가 맞다면,

1. master, develop으로의 PR(기능, 구조 변경사항을 포함한 업무 단위)

2. 업무 브런치로의 PR(기능 또는 구조 변경만 포함된 단위)

을 구분하여 생각해야 할 것 같습니다. 올바르게 이해한 것인지 궁금합니다.

photo
안영회

댓글 6

img

taetaetae

2024-05-30 14:14

안녕하세요. JSON 상하차 스터디 모임에서 해당 질문에 대한 여러 의견을 종합 해봤습니다.


결론은, 코드정리의 크기에 따라 전략을 다르게 가져갈 수 있을것 같습니다.


기능 구현 중 발견되는 작은 규모의 코드 정리나 개선 사항은 동일한 PR 내에서 커밋 단위로 포함될 수 있을 것 같습니다. 이는 반복되는 기능 개발 속에서 코드 베이스를 조금씩 향상시키기 위한 방법으로 작은 규모의 리팩토링에 해당할 것 같구요. 반대로 큰 규모의 리팩토링의 경우는 별도의 PR로 구분하는 것이 좋을 것 같습니다. 큰 리팩토링 작업은 기능 개발과 동일한 PR에 포함될 경우 리뷰 속도와 정확도가 떨어질 수 있기 때문입니다.
"코드 정리"와 "기능 개발"의 규모와 성격에 따라 PR을 구분할 필요가 있다고 생각합니다. 만약 "코드 정리"의 규모가 크다면 이를 별도의 PR로 분리하는 것이 좋을 것 같구요. (피쳐브랜치에서 코드 정리를 위한 하위 브랜치를 새로 만들어서 작업하고 PR 생성) 반대로 크기가 작고 기능 구현 코드와 밀접한 곳에 있다면 커밋단위로 함께 PR을 만들어도 무방할것 같습니다.
"보이스 카웃 룰"을 적용하여 기능 개발을 하는것과 동시에 조금씩 코드 정리를 하는건 좋지만 중요한건, 해당 브랜치/PR의 목적을 달성한 후에 하는 작은 코드 정리는 어느정도 용인이 될 것 같아요!

img

[yeTi] 예티

2024-05-30 23:08

본문의 질문과 teateatea 님의 답변을 보고 생각을 보탭니다.


먼저 책의 제목이 tidy first 입니다.

괴짜들이 세상에서 안전하다고 느끼도록 돕는 일 - p.18


안전하게 설계하는 법을 알려주는 것이 사명이라고 지은이의 말에서 설명하고 있습니다. 그 개념을 간결하게 표현한 것이 tidy first 라고 생각합니다.


PR도 같은 개념으로 접한할 수 있다고 생각합니다. PR의 단위는 리뷰이나 리뷰어가 안전하다고 느낄 수준이어야 하고, 안전하다고 인식되려면 작을수록 유리합니다.

하지만 지금처럼 모든 것을 한꺼번에 모아 놓으면 쉽게 엉망이 되어 버립니다. 그렇게 되면 검토자들을 일단 주저하게 만듭니다. <중략> 그러니 우리는 변경 사항을 나누어 별도의 PR로 만들어야 합니다. - p.73


이러한 맥락에서 본문에서 구조 개선 절차가 선행되어야 한다고 생각한다고 말씀하신 부분은 선/후행의 개념에서 벗어나 상호간 안전하다고 느낄 수 있는 작은 단위의 PR을 만들어야 한다고 생각합니다.


여기서 전체 업무라고 표현하신 부분에서 전체 업무의 단위로 PR을 하게되면 리뷰이는 코드리뷰를 하는데 안전하다고 느낄 수 있을지 궁금합니다. 업무 단위 또한 상호 피드백을 받을 수 있는 단위로 나눠져야 한다고 생각합니다.
예를 들어 지라의 일감이 전체 업무라면 하위 일감을 PR 단위로 만들 수 있겠네요.

img

유영모

2024-05-30 23:26 (수정됨)

PR은 협업 입니다. 상대가 있다는 뜻이죠. 바로 코드 리뷰어입니다.
코드 리뷰어로서 제가 경험한 것에 따르면 PR 크기(코드량)가 매우 중요하다는 것입니다.
저는 작은 PR을 선호하는데 그 이유는 검토해야할 코드양이 많으면 지연(피드백, 출시 등)이 발생하기 때문입니다.
켄트 벡 역시 아래 글에서 PR 크기에 대해 언급하고 있습니다.
https://tidyfirst.substack.com/p/thinking-about-code-review


작은 PR 크기가 어느 정도인지 정하기 어렵습니다. 다만 구글은 코드 리뷰 가이드에서 일반적으로 100줄 정도가 적합하지만 1000줄은 너무 크다고 말하고 있습니다.
https://madplay.github.io/post/small-cls


PR을 작게 만들기 위해서 작업을 '코드 정리'와 '동작 변경'으로 구분하는 것은 유용하다고 생각합니다.
켄트 벡은 책에서는 아래와 같이 말합니다.
"코드 정리와 동작 변경 사이를 번갈아 가면서 전환할 때마다 새 PR을 열어냐 합니다."
논리적으로 동의하지만
실제 실무에 적용할 때에는 내가 처한 맥락에 따라 달라질 수 밖에 없다고 생각합니다.


저라면 아래와 같이 3단계로 할 것 같습니다.


(코드 정리?) -> (동작 변경) -> (코드 정리?)

동작 변경을 하기 전에 코드 정리(구조 변경)를 먼저 하는 것이 도움이 된다고 판단된다면
코드 정리를 할 것입니다.
코드를 정리하고 나니 200줄 정도 되었다고 해보죠.
그렇다면 정리한 코드를 리펙토링 PR로 리뷰어에게 보낼 것입니다.
하지만 코드 정리하고 동작 변경을 모두 했지만 100줄이 체 안된다면 하나의 PR로 보낼 것입니다.
동작 변경에 대한 PR이 끝나면
앞서 @taetaetae 님이 말씀하신 것처럼 "보이스 카웃 룰"을 적용하여 다시 한번 코드 정리할 것이 있는지 보고
코드 정리가 필요하다면 다시 리펙토링 PR을 보낼 것입니다

img

YeongRo Yun

2024-06-12 14:25

안녕하세요, 처음 질문 드린 윤영로입니다.
이제와 생각해보니, 처음 질문을 드렸을 때 PR 개념을 잘못 이해 했었습니다.


저는 PR을 "한 기능의 개발이 끝나면, 그것을 반영하기 위해 요청하는 것"이라고 알고 있었습니다. 하지만 책의 내용과 답변들을 통해 위의 정의보다 "개발의 각 단계가 완료되면, 그것을 검토하기 위해 요청하는 것"이라는 말이 더 맞는 것 같다는 생각이 듭니다.


상세하게 답변 주신 모든 분들께 감사드립니다.

img

안영회

2024-07-26 15:34

@YeongRo Yun 멋지네요.


그리고 답변 주신 @taetaetae @[yeTi] 예티 @Young-mo Yoo 모두 고맙습니다.

img

안영회

2024-08-22 08:14

@taetaetae @YeongRo Yun 이 책에 대한 토론이 진행 중인데 참여해 보세요.


https://tidyfirst.bettercode.kr/posts/21

하고 자유롭게 의견을 남겨 주세요.