Github PR(Pull Request) 생성 전 생각해보면 좋을 것들

kindof

·

2024. 1. 14. 14:10

이번 글은 PR(Pull Request)을 생성하기 전에 한 번쯤 체크해볼만한 내용을 정리해보고자 작성하게 되었습니다.

 

먼저 제가 생각하는 PR의 목적은 크게 두 가지입니다.

 

[1] 이슈를 처리하기 위한 해결책을 담았다.

[2] 코드 리뷰가 필요하다.

 

어떻게보면 당연한 이야기겠지만, 이 두 가지 목적을 담은 PR은 결국 PR을 올린 당사자가 이해하기 쉬운 것보다 PR을 봐야하는 팀원들 혹은 제 3자의 이해가 직관적이어야 효율적인 PR이라는 생각입니다.

 

물론 서비스마다, 팀마다 PR 템플릿도 다르고 브랜치 전략도 다를 수 있기 때문에 제가 생각해 본 체크 리스트가 적용되지 않을 수도 있습니다. 또한 많은 분들은 이미 실천하고 계실 수 있기 때문에 가볍게 봐주시면 좋을 것 같습니다.

 

 

1. PR Checklist

[1] 변경사항을 명확히 설명하는가?

코드의 변경에는 아주 간단한 버그 수정부터 꽤나 큰 기능의 추가/변경까지 수 많은 목적이 있습니다. 

 

따라서 PR에서 가장 먼저 알려야 할 것은 어떤 목적의 달성을 위해 어떤 변경사항이 있었는가를 정의하는 것입니다. 그리고 이 변경사항을 전달하는 데 효과적이라고 생각하는 방식은 Before VS After를 설명하는 것이라고 생각합니다.

 

예를 들어, 기존에는 고객을 조회하는 API의 필터 조건으로 이름이 있었는데 나이대 조건을 추가할 수 있도록 코드의 변경이 있다고 해보겠습니다.

 

가장 기본적인 설명은 아래와 같을 것입니다.

 

- 고객 조회 API 필터 조건 추가 : 나이대

 

그리고 이 설명을 본 제 3자는 생각합니다.

 

- "그래 나이대 별로도 조회할 수 있게 되었구나."

 

하지만 조금 더 구체적으로 설명을 해보면 어떨까요?

  Before After
CustomerSearchCondition private Long customerId;
private String name;
private Long customerId;
private Long name;
private AgeRange ageRange;
AgeRange 없음 나이의 범위를 스캔하는 Enum 생성

 

간단한 변경이라 큰 차이는 없어보이지만 제 3자의 느낌은 조금 다를 것이라고 생각합니다.

 

- "나이대를 정의하는 Enum 클래스를 만들고 조회 조건에 넣어서 필터링하는구나."

 

변경사항의 전/후를 비교해서 설명할 때는 문장으로 풀어서 설명할 수도 있고 테이블로 정리할 수도 있고 그림이나 사진을 사용할 수도 있습니다.

 

좀 더 효과적인 방식이 무엇일지는 개인마다 선호가 다를 것 같지만, 위와 같은 방식으로 변경 포인트의 Before VS After를 설명하면 제 3자로 하여금 실제 변경된 코드를 볼 때 보다 나은 예측 가능성을 제공할 수 있으리라 생각합니다.

 

 

[2] 다른 작업, 혹은 앞으로의 작업들에 영향이 있는가?

특정 클래스 혹은 API 등을 Deprecated 하는 작업이 있다고 해보겠습니다. 하지만 당장은 사용하는 곳이 있기 때문에 코드를 지우지는 못하는 상황입니다.

 

이 작업에 대한 PR에서는 반드시 다른 작업자들로 하여금 해당 클래스를 앞으로 변경하지 말 것과 해당 클래스 대신에 새로운 클래스를 사용할 것을 당부해야 합니다.

 

뭔가 '굳이 이런 말을 넣어야 할까?' 싶기도 한 고민이 들 때도 있겠지만 간단한 코멘트정도 남긴다면 득은 적을지언정 실은 없다고 생각합니다.

 

 

[3] 불필요한 코드가 남아있지 않은가?

가장 기본적이면서도 자주 실수하는 부분입니다.

 

여러 커밋을 남기면서 최종 PR에서는 포함되지 않아야 할 코드가 남아있는 경우가 많습니다.

대표적인 예시가 불필요한 줄바꿈, 사용하지 않는 Import문과 의존성들, 미처 삭제하지 않은 파일 등이 있습니다.

 

이런 코드들은 마치 사진을 찍을 때 주변에 시선을 분산시키는 물건이 놓여져있는 것과 같습니다.

 

PR 생성 전, 코드의 변경사항을 보며 이런 흔적이 남아있는지 꼭 확인하면 좋을 것 같습니다.

불필요한 코드

 

 

[4] 연관없는 코드가 포함되어 있지는 않은가?

위에서 말한 '불필요한 코드'와는 조금 다른 내용인데요.

 

A 작업을 위해 코드 수정을 하다가 갑자기 예전에 아주 조금 수정하면 좋을 것 같다고 생각한 코드가 떠올라 이를 고치는 경우가 있습니다. 혹은, 갑자기 전혀 연관이 없는 파일을 우연히 열었다가 사소하지만 못마땅한 코드가 보여서 이를 수정할 수도 있습니다.

 

작업과 연관이 있는 코드를 조금 손보는 것은 어느정도 괜찮다고 생각하지만, 전혀 연관없는 포인트를 수정하는 것은 그 수정이 합리적일지라도 PR을 이해하는 데 방해가 될 수 있다고 생각합니다.

 

 

[5] 이게 최선인가?

마지막 내용이 가장 중요한 것 같은데요.

 

과연 이 코드 변경이 최선인가 스스로 검토해보는 것입니다. 작업이 막바지에 이르게 되면 뭔가 빨리 이 작업을 마무리하고 싶다는 생각에 아주 사소한 내용을 놓칠 때도 있고, 조금 더 신경쓸 수 있는 부분을 놓치는 경우가 있습니다.

 

아주 급한 일이 아니라면 일부러라도 여유를 갖고 잠시 후에 PR을 생성해보는 것을 추천드립니다.

 

그러면 문득 떠오르는 수정사항이 생길 때가 있는 것 같습니다.

 

 


 

 

지극히 주관적인 내용들이지만, PR을 생성할 때 머릿속으로만 되뇌여보는 내용들을 글로 적어봤습니다.

 

끝!