이번주 토요일부터 개발자 영어 스터디를 참가하게 되었다.
방식은 주어진 아티클을 읽고, 내용과 모르는 표현 등을 숙지하고 정리하는 것!
토요일 아침 7시가 조금은 부담스럽긴 하지만, 이왕 하는거 열심히 해보자
첫 스터디 기사는 코드리뷰에 관한 글이다.
Code Reviews At Medium - Small PRs, quick reviews, and working like we're in this together
나는 아직 회사에서 코드리뷰를 해보거나 받아본 경험이 없기에... 그리고 다음주에 새로운 직장으로 이직을 하기에 나한테 더 필요한 글이다.
Engineering teams have different norms and policies when it comes to code reviews. It can be frustrating to join a team and not know what is expected, or to operate with old assumptions and discover that they are at odds with a new team. ......
엔지니어링 팀들은 코드리뷰에 있어 서로 다른 표준과 정책을 가지고 있다. 새로 팀에 들어가게 되어 팀이 뭘 기대하는지 모를 때가 있다. 아니면 오래된 가정하에 움직였는데 그것이 새로운 팀과 상충하는것을 발견한다. ......
Medium에서는 이러한 불편함을 겪는이들을 위해 다음과 같은 노하우를 전수했다.
Changes require at least one approval
At Medium, engineers have to get their code approved by at least one other engineer before committing it to the codebase. We enforce this requirement with settings in our Github repository.
한명 이상이 승인해야 PR이 반영 되는 룰을 정했다.
이는 repository를 보호하고, PR 요청을 검토하게 만드는 효과가 있다.
깃헙 Managing a branch protection rule을 보면 자세한 설명이 나와있다.
Reviewers
In general, we try to to make sure that engineers are working in pairs or groups on projects, and one of the benefits is that engineers have built-in reviewing buddies. If an engineer is working on a project like this, it’s natural for them to tag these people on their PRs.
일반적으로, 짝을 짓거나 그룹으로 프로젝트를 하게 되는데 이들을 보통 Reviewer로 지정하곤 한다.
만약 혼자 떨어져서 일하게 된다면, git blame을 통해 해당 파일의 작업을 최근에 누가 했는지 확인하고 그들이 어떻게 일했는지에 대한 관점에 대해 파악을 한다.
Help me help you
Anyone who is reviewing code will need to figure out what is changing and why it’s changing. To facilitate this, engineers do often add descriptions, either in the PR itself or in a comment. In general, we think it’s good to consider what people may or may not know about your PR, and aim to give them context so that they can help you.
코드리뷰를 요청받는 사람은 무엇이 바뀌었고, 이것이 왜 바뀌었는지 알 필요가 있다.
이것을 가능하게 하려면 PR 혹은 변경된 코드에 주석을 달아야한다. 이러한 것들을 리뷰어들에게 전달해주어 그들이 나를 도울 수 있게 만들어야 한다.
Quick reviews
Most of my PRs get reviewed within a couple of hours. We recommend that a PR should be reviewed within 4 hours. This guideline doesn’t mean that every reviewer has responded within a few hours; it means that at least one reviewer has responded within a few hours.
Medium팀은 작은 PR과 빠른 리뷰를 중요하게 생각하고 권장한다고 한다.
대부분의 PR들은 몇시간 안에 리뷰를 거치게 된다.
Medium은 4시간 안에 리뷰가 이뤄지는 것을 추천한다.(모든 리뷰어들이 그렇게 하는 것이 아니라 한명 이상은 1~2시간 안에 리뷰를 해야한다는 뜻이다)
PTAL
PTAL stands for Please Take A Look or Please Take Another Look. When this comment is added, our Slack integration tool will send a notification to all reviewers. The notification will tell them that the author is asking them to “please take another look.”
만약 몇시간이 지나도 리뷰가 되지 않는다면 어떻게 해야할까?
아니면 리뷰가 이루어져서 변경해야하거나 수정해야할 피드백이 생겼고 또 다른 리뷰가 필요할때는?
Medium은 이럴 경우 "ptal"이라고 PR에 코멘트를 남겨두거나 ⚡와 같은 번개모양 이모티콘을 남겨둔다고 한다.
ptal은 Please Take A Look 또는 Please Take Another Look 을 뜻한다고 한다.
이를 커멘트에 추가하면 슬랙에 새로운 안내메세지가 전송되게 되고 리뷰어들은 이를 다시 볼 수 있게 된다.
따라서 "ptal"을 쓰면 리뷰가 필요한 PR이란 것을 같이 일하는 엔지니어들에게 전달할 수 있는 것이다!
Beyond PTAL
In general, we think of the author as being responsible for getting the review that they need. We ask engineers to remember that other engineers want to help and be responsive, but some days and some workloads don’t allow for it. That’s why it’s good to escalate when someone is sensing that their PR has slipped through the cracks.
ptal 또는 ⚡ 을 썻는데도 리뷰가 없다면...??
그런데 하필 이 PR이 급한 사항이거나 혹은 다른 PR보다 조금 더 빨리 리뷰가 필요하다면 어떻게 될까?
이럴때 escalate 해야한다고 하는데 아마도 직접적으로 나서는 것을 의미하는 것같다.
예를 들어 슬랙채널에 PR을 올려서 리뷰를 해달라고 요청을 하거나 리뷰어들에게 직접 요청을 하는 방법들이 있다.
escalate에 대한 의미는 보강 필요!
Paradoxically, small PRs > big PRs
At Medium, we strive to build things in small units. We want to break our code down as much as possible and submit small, digestible PRs. By doing this, we can review quicker and move faster.
small PR이 big PR보다 낫다!
small PR과 big PR을 나누는 기준은 무엇일까?
변화되는 양을 기준으로 하는데 여기서 변화의 기준은 코드라인 수를 의미하는것이 아닌 개념적인 것의 변화를 의미한다.
We try to see things from other people’s perspectives
When we’re reviewing code, we don’t act like gatekeepers; we aim to help our co-workers do what it is they’re trying to do.
다양한 관점을 보아야 한다는 의미다.
정확하기가 두려워 코드리뷰를 못하는 것보단, 확신이 없어도 자유롭게(적당한 선에서) 모든 인원들이 리뷰를 할 수 있어야하고 모두를 도울 수 있는것이 최종 목표가 되어야 한다는 의미이다.
코드리뷰가 절대적으로 깐깐해져서 "이 코드는 통과되어도 좋습니다. 이 코드는 안됩니다." 라고 판독하는 판독가가 되는 것이 아닌 동료와 나 자신을 위한 코드 리뷰문화를 갖춰야 한다!
또한 합리적인 사람이 되어야 한다.
코드 리뷰를 하는 사람의 의도가 그 사람 나름의 합리적인 의도를 가지고 이루어졌다고 생각을 해야한다.
(공격하려는 의도가 아님을 인지해야 한다)
이러한 자세로 리뷰어를 사람대 사람으로 대하고 관대하게 그 사람의 의도를 해석할 줄 알아야 한다!
norms
보통, 일반, 표준, 규범
In an effort to make it easier
수월하게 하기 위한 노력으로
escalate
차오르다, 단계적으로 확대되다
consensus
의견 일치
explanatory
설명
interpretations
해석
intentions
의도
