동료의 PR을 리뷰하다가 폼 컴포넌트 안의 검증 로직이 길어 보였다. 커스텀 훅으로 빼면 테스트하기 쉬울 것 같지만, 지금 PR에서 꼭 해야 하는 일은 아니다. 한국어로는 "이 부분 훅으로 분리해 주세요"라고 남길 코멘트다. "주세요"를 공손하게 옮기면 Please가 떠오른다.
| 용어 | 뜻 |
|---|---|
| 주어(subject) | 문장에서 동작을 하는 사람이나 사물. 우리말의 '~이/가'에 해당한다. 예: CI runs the tests on every push.의 CI |
| 조동사(modal verb) | must, should, could, might처럼 다른 동사 앞에 붙어 의무·조언·가능성 같은 뜻을 더하는 말. 예: This test might fail on Windows. |
| 명령문(imperative sentence) | 주어를 빼고 동사의 기본 꼴로 시작해 상대에게 하라고 말하는 문장. 예: Run the linter before you push. |
| 의문문(question) | 묻는 문장. 흔히 조동사나 be, do를 주어 앞으로 옮겨 만든다. 예: Is the build green? |
| -ing 형(-ing form) | 동사 끝에 -ing를 붙인 꼴. 명사처럼 쓰여 문장의 주어가 되기도 한다. 예: Caching the response cut the load time. |
꼭 고칠 필요는 없는 제안으로 읽히는 코멘트는?
규칙: 조동사와 문장 형태로 강도를 정한다
A. 조동사마다 강도가 다르다. British Council은 must를 꼭 필요한 일에, should를 제안과 조언에, could를 가볍게 권하는 제안에 쓴다고 설명한다. 같은 페이지의 British Council 답변은 should는 선택이 있다는 뜻을 담고 must는 선택이 없다는 뜻을 담는다고 덧붙인다(British Council: Suggestions and obligations). 리뷰 코멘트로 옮기면 대략 이렇게 나뉜다.
- 반드시:
This must be fixed before merge./This needs a null check. - 권장:
You should add a test for this branch. - 제안:
You could use the existing date util here.
같은 답변란에서 British Council은 You might want to …도 제안을 하는 꽤 흔한 방법이라고 한다. might want to는 '이렇게 하고 싶어질 수도 있다'는 말이라, 판단을 작성자에게 넘기는 부드러운 제안이 된다.
You might want to extract this into a hook.
명세 문서의 MUST, SHOULD, MAY는 이런 어조가 아니라 정해진 뜻으로 읽는다. 그 이야기는 Unit 5에서 다룬다.
B. 질문으로 제안한다. 문장을 질문으로 바꾸면 작성자가 답할 자리가 생긴다. 의도를 모를 때는 고치라고 하기 전에 이유부터 묻는다.
Could we reuse the existing modal here?What do you think about splitting this into two PRs?Is there a reason this list isn't memoized?
C. 접두어로 강도를 밝힌다. Google의 코드 리뷰 가이드는 코멘트에 등급을 붙여 꼭 고칠 것과 제안을 구분해 보라고 권한다(Google eng-practices: How to write code review comments). 가이드의 정의는 이렇다.
Nit:사소한 것. 하는 편이 맞지만 큰 영향은 없다.Optional:(또는Consider) 좋은 생각일 수 있지만 꼭 필요하지는 않다.Consider adding a test for the empty list.처럼Consider뒤에는to add가 아니라adding처럼 동사의 -ing 형(-ing form)이 온다(Longman: consider).FYI:이번 변경에서 하라는 것이 아니라 앞으로 참고할 만한 정보다.
D. 사람이 아니라 코드를 주어(subject)로 쓴다. 같은 가이드는 개발자가 아니라 코드를 두고 말하라고 한다. 가이드의 예도 "왜 여기서 스레드를 썼느냐"는 질문 대신 "여기의 동시성 모델이 성능 이득 없이 복잡도만 늘린다"처럼 코드를 주어로 쓴다.
You forgot to clear the timer.대신The timer isn't cleared on unmount, so it can fire after the component is gone.You made this too complex.대신This could be simpler with an early return.
리뷰에서 무엇을 보고 어떤 기준으로 승인할지는 코드 리뷰 노트에 있다. 이 유닛은 그 판단을 영어 문장으로 옮길 때의 강도만 다룬다.
표현: 강도별 리뷰 코멘트
| 표현 | 언제 | 예 |
|---|---|---|
This needs to … / must | 머지 전에 반드시 고칠 때. 이유를 같이 쓴다 | This needs to handle an empty array. It crashes on an empty cart. |
You should … | 고치는 편이 맞다고 권할 때 | You should add a test for the error path. |
You could … / You might want to … | 다른 방법을 제안할 때 | You might want to extract this into a hook. |
Consider …ing | 선택 사항으로 제안할 때 | Consider using the existing date util. |
Could we …? / What do you think about …? | 함께 정하자고 할 때 | What do you think about moving this to a separate PR? |
Is there a reason …? | 의도를 모를 때 | Is there a reason this is typed as any? |
Nit: / Optional: / FYI: | 강도를 앞에서 밝힐 때 | Nit: extra blank line. |
This needs to …, 제안은 You might want to …, Could we …?, Optional:처럼 강도가 문장 형태에 드러납니다.Please extract …로 옮기면 공손한 명령문이 되어, 제안으로 남긴 코멘트가 머지 조건처럼 읽히기 쉽습니다. '이거 왜 이렇게 했어요?'를 Why did you …?로 옮기면 사람을 따지는 말로 읽히기 쉽습니다. 제안이면 Could we …?, 이유가 궁금하면 Is there a reason …?으로 바꿉니다.함정 찾기: AI가 옮겨 준 리뷰 코멘트
내 리뷰 코멘트 다섯 개를 영어로 옮겨 줘.
- 사소한 것에는 Nit을 붙이기
- 나머지도 부드럽게
의심스러운 줄을 모두 눌러 표시한 뒤 “검토 끝”을 누르세요. 함정은 2개입니다.
연습
1. 이번 PR에서 고치라는 것이 아니라 앞으로 참고할 정보만 남길 때 붙이는 접두어는?
2. 빈 배열이 들어오면 앱이 멈춘다. 머지 전에 반드시 고쳐야 할 때 코멘트는?
3. 타이머 정리를 빠뜨렸다는 지적을 사람이 아니라 코드를 주어로 쓴 것은?
4. 작성자의 의도를 모른다. 탓하지 않고 이유를 묻는 질문은?