최근 들어 꼼꼼하신 분들과 일을 하다 보니 PR 요청에도 많은 신경을 쓰게 된다.
정리되지 않은 머릿속의 관행과 지식 정도로 PR을 작성하곤 했는데, 글로 정리해두는 것도 좋을 것 같다.
* PR : Pull Request
* 보이 스카우트 규칙 : 수정 전보다 수정후 더 코드는 깔끔하게 유지되어야 한다는 규칙
1. 작게 유지할 것
PR의 규모는 작게 유지하는 것이 좋다. 개발하면서 코드를 작성하고 리팩토링 하고, 포맷팅, 보이 스카우트하는 것이 매우 유혹적이지만, 일반적으로 최고의 개발자들은 모든 것을 한 번에 바꾸고 싶은 유혹에 대해 저항한다. 능숙한 개발자들은 단일 목표를 유지하고 최소한의 코드 변경으로 작업을 완료한다. 일부는 심지어 '줄 삭제', '줄 추가' PR 비율을 가장 높게 하려고 노력한다. 보이 스카우트나 리팩토링이 필요하다면 PR을 따로 생성하는 것이 좋다. 모든 것을 하나의 PR에 집어넣는 것이 왜 더 쉬운지에 대한 변명을 만들지 말자. 변명이 아니라 게으름일 뿐이다. 대신 더 작은 PR을 만들 수 있는 방법을 발명해보자. 그게 창의력이다.
2. 셀프 리뷰를 수행하자.
초안 PR을 작성하고 주석과 작업을 포함하여 전체 셀프 리뷰해보자. 코드 작성을 끝낸 후 PR에서 변경 사항을 버리고 다른 사람들이 실수를 찾도록 하는 게 더 편하고 유혹적이다. 특히 며칠이 걸린 큰 변경사항일 경우 더욱 그렇다. 하지만 나태해지지 말자. 규칙을 지키자. 내 코드는 내가 작성해야 한다. 다른 사람들에게 의존하지 말자.
또한, "셀프리뷰"를 사용하여 문제를 지적할 수도 있다. "이 이름에 대해 잘 모르겠는데, 더 나은 이름이 있나요?", "다른 분들은 어떻게 생각하나요" 같은 질문을 쓰는 동안 실제로 스스로 대답할 수 있고, 자기 성찰이 자기 성차리 스스로의 일상적인 코딩 사고 과정이 몸에 내장되게 된다. 즉 다시 말해, 이러한 셀프 리뷰 과정은 스스로를 더 나은 개발자로 만들게 된다.
3. 돌아가기 그리고 노이즈 제거하기
셀프 리뷰 중에 공백 변경, 서식 변경 또는 PR의 의도와 관련이 없는 일부 텍스트 변경만 있는 파일을 발견하는 경우가 많다. 다시 롤백하고 이전 상태로 돌아가자. 파일을 약간 개선한 것은 문제가 되지 않는다. 기능적으로는 동일하며, PR 목록에 나열된 'Changed files' 목록에 있는 추가 파일은 Reviewer의 고통을 유발하고 적절한 검토에 대한 동기를 낮춘다.
포맷이 중요하다면 별도의 PR을 만들고, 필터를 도입하여 CI의 일부로 만들고, 하나의 큰 PR로 전체 앱을 포맷하자. 노이즈는 중요한 변화와는 분리되어 있어야 한다. Reviewer의 시간과 노력을 존중하자.
노이즈들은 Git history를 오염시켜서 특정 파일의 변경 의도를 숨긴다. 그리고 그 history를 더 탐색하기 어렵게 만든다.
4. 의미 있는 Title을 만들자
종종 제목은 브랜치 이름이나 티켓에서 생성된다. 여기서 유일한 규칙은 어떤 종류의 규칙을 따르고 제목을 짧고 의미 있게 만드는 것이다. 브랜치의 이름을 짓는 것과 유사하게 생각하면 된다.
Pull Requests는 새로운 문서이다. 즉 PR 기록을 쉽게 찾아볼 수 있어야 하고, 과거의 의사 결정 및 생각 프로세스를 훨씬 쉽게 검색할 수 있어야 한다. 그러므로 Title이 중요하다.
5. 의미있는 설명을 작성하자
다시 한번 강조하자면, PR 기록은 매우 유용한 정보이기 때문에, 실제로 읽을 수 있는 설명서로 취급된다. 가능한 한 철저하되, 간결하게 설명하자. 개발자의 의도와 의사결정 과정에 대해 가능한 한 투명하게 함으로써 토론을 선점하자.
영감을 촉진하는 유용한 방법은 PR 템플릿을 설정하는 것이다. 템플릿의 내용은 시간이 지남에 따라 팀과 합의하고 개발/조정해야 하지만 우선 다음과 같은 몇 가지 항이 있다.
Overview of changes : 여기서 다루어야 할 사항은 중 하나는 PR에 없는 것, 즉 평가하여 폐기한 대안이다. 이렇게 하면 이미 시도한 내용을 제안할 수 있는 검토자와의 잠재적인 discuss가 생략될 수 있다.
Questions/Notes for reviewers : 코드에서 구체적인 조언이 필요한 부분이 있다면 작성해보자. 당신이 어떤 것의 이름을 리팩토링 했고 이것은 많은 파일들을 건드렸지만 기능적인 영향이 없는 경우를 생각해보자. 많은 파일 변화에 대해서 안전하다고 reviewer에게 알려준다면 리뷰어는 리뷰에 도움이 될 것이다.
How to test/demonstrate : 준비 환경, configuration/preparation/setup instructions 등이나. 데모해볼 수 있는 user/passwd 같은 정보들이 해당된다.
첨부파일(스크린샷, 동영상) : 사진 한 장이 백만 단어보다 더 큰 정보를 가진다. 변화를 시각적으로 보는 것을 대신할 수 있는 것은 없다.
6. 모든 코드를 보고 모든 코멘트를 신경 쓰자
비동기 커뮤니케이션의 경우, 모든 내용을 다 봤다는 코멘트가 매우 중요하다. 간단한 이모지라도 좋다. 특히 새로운 팀에서는 아무리 작더라도 코멘트를 무시하지 말자. 일단 팀과의 공감대를 형성하고 나면 몇 가지는 그냥 넘어갈 수는 있다. 하지만 적어도 처음에는 예절을 차리고 귀족의 언어를 말하고 예의를 바르게 행동하자.
7. 승인할 때까지 Merge하지 말자.
매너는 사람을 만든다. 매너에 대해 말하자면, 제안을 한 모든 사람들이 당신의 반응을 듣고 평가할 때까지 기다리는 게 좋은 매너이다. 비교적 오래 기다린 경우, 이메일, 메시지 등의 간단한 메시지를 보내고 리뷰를 기다리는 중 임을 알려주자.
많은 멤버로 구성된 팀이라면 모든 PR을 기다릴 필요는 없다. 각 PR을 검토해야 하는 사람들을 결정하기 위한 시스템을 마련하자. 아마도 특정 사람들이 특정 모듈을 책임지고 있을 것이다. 라운드 로빈 방식도 있고, 일을 나누는 방법도 있다. 창의적으로 책임을 나누자.
Review
아래 내용부터는 Reviwer에게 해당되는 내용이다.
8. Check out 하자
항상 한 번에 두 개의 프로젝트 클론을 PC에 저장하자. 하나는 정상적인 업무이고, 하나는 PR을 검토하는 것이다. 이렇게 하면 오버헤드 없이 현재 작업을 일시 중지할 수 있다.
브랜치를 체크아웃하고 빌드/테스트 실행 후 다시 정상적인 업무를 하는 브라우저로 돌아가자.
9. 제목과 설명을 읽자
만약 누군가가 자신의 PR에 대한 가이드를 쓰려고 노력을 했다면, 최소한 당신이 할 수 있는 것은 시간을 내서 읽는 것이다. 관련된 티켓, PR 제목 및 설명을 읽고 리뷰하려는 내용에 대한 기대치를 높이자.
10. 로컬에서 검증하기
개선할 수 있는 사항이 발견되면, 코멘트를 하기 전에 로컬 클론을 변경해보자. 프로젝트를 처음 시작할 때 특히 중요하다. 불가능하고 컴파일도 안 되는 코드 변경을 제안하는 것보다, 더 창피한 일은 없다. 하지만, 당신은 IDE에 코드가 있을 때 더 좋은 좋은 영감을 받고, 실행에 실패하면서 헛수고였다는 걸 알게 되는 때가 많을 것이다. 제안을 검증하는 것도 중요하고 가치 있는 이리다.
일단 당신이 제안하려는 것이 최소한 가능하다는 것을 확인했으면, 당신의 노력을 낭비하지 말고, 코드 변경사항을 복사하고 그것을 코드 작성자가 분기에 바로 복사할 수 있도록 코드 블록을 사용해 코멘트하자.
11. 대규모 제안을 PR으로 생성
만약 당신의 제안이 꽤 커졌다는 것을 알았다면, 당신의 노력을 낭비하지 말자. 작성자의 브랜치를 분리하고 원래의 PR에 병합할 PR을 만들자. 새로운 PR에서 더 큰 변화에 대한 토론을 위한 별도의 장을 제공하고 이 PR에는 원작자도 참여할 수 있다. 모든 것이 다 처리된다면, 메인 PR에 병합시키면 된다.
12. 코멘트를 달고 싶은 충동을 참자
코멘트를 달면서 가장 힘든 것은 코멘트를 다는 충동을 참는 것이다. 다른 사람에게 엄격하기 전에 자기 자신에게 먼저 엄격해져라. 당신이 리뷰어로서 얼마나 가치를 가지는지 궁금하다면 당신의 모든 코멘트를 다시 살펴보고 얼마나 많은 코멘트가 좋은 평가를 받고 최소한으로 참고되고 이행되는지 관찰해보자.
다음은 코멘트를 다는 방법이다.
13. 구체적인 대안이 없다면 코멘트하지 말 것
단순히 마음에 들지 않는 것이 있다면, 더 나은 것을 생각해 내고 당신의 주장을 펴라, 그렇지 않다면 손가락을 묶어두자.
❌ 이렇게 작성한 게 좀 이상한데요?
❌ 이런 식의 접근은 좋지 않아요.
✅구체적인 대안을 말하자.
14. 게을러지지 말고 자신감을 가져라
"아마도", "모르겠다", "만약에" 같은 의심을 내포하는 단어들을 사용하지 말자. 잘 모르겠으면 반성하자. 그리고 몇 가지 테스트를 한 뒤에 자신감을 가지고 돌아오자. 코드를 작성하는 것보다 테스트하는 것이 더 어렵다. 남에게 어려운 일을 미루지 말자.
❌ 잘 모르겠지만, 이렇게 바꾸면 더 좋을지도 몰라. 확인해보고 어떻게 생각하는지 말해줘.
❌ 게으름뱅이라면 알 때까지 코멘트하지 말자.
✅ 알고 있다면 모른다고 말하지 말자. 자신감을 갖고, 자신의 의견을 제안하고, 왜 그것이 더 나은지 명확하고 개략적으로 설명하자.
✅ [Code block]과 같이 인라인 기능으로 리펙토링 하면 코드가 작아지고 관용적이 됩니다. 또한 코드 중복을 방지합니다.
15. 스타일 규칙을 변경하기 전에 따라 하기
사람들은 이상하게 자신의 스타일에 애착을 가지고 있고 매우 고집스럽다. 새로운 팀에 합류할 때, 기존 구성원들은 종종 프로젝트의 현 상태를 옹호할 것이고, 새로운 팀원들을 종종 그들이 "이전 프로젝트에서 어떻게 더 잘/다르게" 했는지를 설명한다.
조금 더 큰 관점에서 보자.
기존 스타일에 적응하면 새로운 팀과 함께 아키텍처 결정, 패턴 및 관행과 같은 더 중요한 사항에 대한 제안 상항에 대해 보다 개방적인 태도를 취할 수 있다. 불만 없이 스타일을 익힌 후 스타일 현대화에 대한 힌트를 준다면, 조금 더 수용적인 상대 개발자를 발견할 수 있을 것이다.
16. 트집을 잡는 것은 좋은 신호이다
만약 당신의 동료들이 당신의 PR을 검토하고 당신이 스타일 수정 코멘트만 받는다면 그것은 좌절감을 주는 트집잡기처럼 보일 수 있다.
하지만 이렇게 생각해보자 : 리뷰어들은 내 코드에서 실질적인 문제를 찾기 위해 애쓰고 있다.
트집잡기에 대한 일부 좋은 반응은 모든 스타일 결정의 90%를 자동으로 처리하는 IDE 도구가 있을 때 트집잡기가 얼마나 비효율적인지 정중하게 강조하는 것이다. 보통 자동화된 스타일 체크가 존재하지 않을 때 그것은 게으름, 동기부여의 부족 증상이다. 그러므로 스스로 만들자.
사람들이 종종 놓친다면 git action CI봇을 통해 코멘트를 달수도 있다.
17. 회의실로 Thread를 가져가자
당신의 경력의 어느 시점에, 당신은 화가 나서 열심히 코멘트를 달수도 있다. 그럴 때는 일단 대화를 멈추고, 진정하고, 다음과 같이 이메일이나 메시지를 보내자.
안녕하세요.
PR에서 벌어지고 있는 댓글 전쟁에 대해 사과드립니다. 상황이 좀 삐뚤어지고 있습니다. 이제 회의실에 모여서 직접 논의해볼 필요가 있을 것 같습니다. 서두를 필요가 없으니 천천히 대답해주세요. 잘 생각해보고 답장 주세요. 내일 미팅 가능합니다.
사람들은 온라인에서 보는 것만큼 직접 만났을 경우 잔인하거나 무례하지 않다. 당신의 요점과 반대의견을 잘 생각해서 회의를 냉정하게 진행해보자. 객관적으로 판단하자. 최선을 해결책을 찾는 것이 목표이지, 내 의견을 상대에게 관철시키는 게 목표가 아니다.
18. 뉘앙스를 좋게 유지하자
PR을 작성하는 것은 기본적으로 코멘트와 비판을 불러온다. 그래서 무엇보다도 리뷰어들은 비판적이어야 한다. 하지만 개인적으로 가 아니라 전문적이어야 한다.
분위기를 위해 이모티콘을 사용해보자.
✅ 와우 신기능을 사용했네요 🤩
You를 피하고 We를 사용하자.
❌ XX님 여기 변수 선언을 빠뜨렸네요.
✅ 우리가 여기 변수 선언을 빠뜨린 것 같은데요?
아니라면 차라리 수동적 표현을 쓰자.
✅ 여기 누락된 변수가 있습니다. [Code block]
댓글 해석에 유의하자. 대부분은 작성자가 나쁜 사람이기에 코멘트를 딱딱하게 단것이 아니다. 대부분 코멘트는 실제 발화되는 말보다 더욱 딱딱하게 느껴진다.
19. 긴급 PR을 피하자
PR은 다음과 같은 두 가지 이유 때문에 요청된다.
1. 비동기 의사소통 : 개발자가 언제든지 검토 및 대응을 할 수 있으므로 흐름의 자연스러운 중단이 발생할 때 중단 없이 개발을 계속하고 PR을 검토할 수 있다.
2. 품질 개선 : 코드가 브랜치에 도달하기 전에 검토하고 테스트하면 브랜치가 깔끔하게 유지된다. 팀원들은 매일 작업하는 코드에서 잠재적인 문제를 발견한다.
긴급 PR은 위 두 가지 목적을 모두 무시하는 행위이다. 만약 10분 안에 브랜치를 Merge 해야 한다면 지금 당장 검토하라는 메시지를 보내지 말고 Merge 하자. 단지 프로세스를 위해 PR을 신경 쓰지 말자. 짧은 시간 내에 병합해야 하는 브랜치가 있는 경우, 한 사람 불러 페어 프로그래밍을 수행하거나, 마음대로 병합해서 품질을 저하시키거나 두 가지 중 하나를 하면 된다.
Pull request는 프로젝트에서 비교적 표준으로 자리 잡은 약속이다. 여전히 새로운 프로젝트마다 새로운 기술과 프로세스를 배우고 있다. 위의 조언이 모든 프로젝트에서 적용되는 경우는 거의 없다. 엄격한 규칙과 프로세스를 갖추는 것보다 팀으로서 협력하는 것이 더 중요하다. 유연하고 친절해야 하지만, 자신감 있고 절제되어야 한다. 서로 주고받고 선생님과 학생이 되자.
변화에 대한 당신의 생각이 선의 근거, 모범을 보인다면, 사람들은 당신을 따를 것이다.
'ETC' 카테고리의 다른 글
| 170422 조금 이상한 큐브 (0) | 2021.03.30 |
|---|---|
| 210327 (0) | 2021.03.27 |
댓글