일 | 월 | 화 | 수 | 목 | 금 | 토 |
---|---|---|---|---|---|---|
1 | 2 | 3 | 4 | |||
5 | 6 | 7 | 8 | 9 | 10 | 11 |
12 | 13 | 14 | 15 | 16 | 17 | 18 |
19 | 20 | 21 | 22 | 23 | 24 | 25 |
26 | 27 | 28 | 29 | 30 | 31 |
- nginx
- 인텔리J
- hibernate
- Spring Batch
- elasticsearch
- Storm
- Spring XD
- hadoop
- intellij
- SBT
- Java
- Linux
- design pattern
- Angular2
- Gradle
- 제주
- docker
- apache storm
- DDD
- Clean Code
- 도메인주도설계
- elastic search
- Spring Boot
- 스프링 배치
- Hbase
- 엘라스틱서치
- Spring
- spark
- hdfs
- scala
- Today
- Total
욱'S 노트
구글 코드 리뷰가이드 요약 본문
작성자
1. 좋은 CL 설명 작성하기
기능변경
RPC 서버 메시지 빈칸 목록에서 크기 제한을 제거합니다.
FizzBuzz와 같은 서버에는 매우 큰 메시지들이 있고 재사용 이점이 있습니다. 빈칸 목록을 더 크게 만들고 시간이 지남에 따라 천천히 빈칸 목록을 해제하는 고루틴(goroutine)을 추가합니다. 따라서 유휴 서버는 모든 빈칸 목록을 해제합니다.
리팩토링
TimeKeeper를 사용하여 TimeStr과 Now 메서드를 사용하도록 태스크를 구성한다.
태스크에 Now 메서드를 추가하여, borglet() 접근자 메서드를 제거할 수 있습니다. (OOMCandidate에서 borglet의 Now 메서드 호출하는데만 사용) 이것은 TimeKeeper에 위임하는 Borglet 메서드를 대체합니다. 결국, Now 메서드를 가져오는데 의존하는 것들은 TimeKeeper를 직접 사용하도록 변경되어야 하지만 리팩토링하기 위한 것입니다. Borglet Hierarchy를 리팩토링하는 장기 목표를 계속합니다.
첫 번째 줄에는 CL이 하는 것과 과거와 어떻게 다른지 설명합니다.
- 수행중인 작업에 대한 간략한 요약
- 명령문처럼 쓰여진 완전한 문장
- 바로 뒤에 줄 바꿈
나머지 설명에서는 특정 구현, CL의 문맥, 현재 해결방안이 이상적이지 않으며 향후 방향에 대해서 설명합니다. 또한 이 변경을 진행하는 이유에 대해서도 설명합니다.
2. 변경사항을 작게 나누기
CL의 적절한 크기로는 하나의 독립된 변경사항만 있어야 합니다.
- CL은 단지 하나의 변경만 다루는 최소한의 변경사항입니다.
- 리뷰어가 CL에 대해서 알아야하는 모든 내용은 CL과 CL 설명문 그리고 코드 베이스 또는 이미 검토한 CL에 있어야 합니다.
- CL을 반영한 후에도 빌드와 시스템이 잘 작동해야 합니다.
- 새로운 API를 추가할 때에는, 그 API를 사용하는 곳도 동일한 CL에 포함
- 리팩토링은 기능 변경이나 버그 수정과 별도의 CL에서 진행하는 것이 가장 좋습니다.
- 관련된 테스트 코드는 동일한 CL에 포함
큰 CL이 허용되는 경우
- 전체 파일을 삭제하는 것은 한 줄의 변경으로 간주할 수 있습니다. 리뷰어가 검토하는데 오래 걸리지 않기 때문입니다.
- 완전히 신뢰할 수 있는 자동 리팩토링 도구에 의해 생성된 코드도 포함됩니다만 머지 또는 테스트에는 위의 일부 지침이 적용됩니다.
3. 리뷰어의 의견을 다루는 방법
개인적인 감정으로 잇지마라
- 리뷰의 목표는 코드와 제품의 품질. 사람에 대한 공격이 아님
코드를 수정한다.
- 리뷰어가 코드의 명확하게 이해하지 못한다는 것. 코드 자체가 명확하지 않다는 것. => 구글은 훌륭한 소프트웨어 엔지니어를 고용하며 당신은 그 중 하나입니다. 당신이 코드를 이해하지 못한다면, 다른 개발자도 코드를 이해하지 못할 가능성이 높습니다.
- 코드를 명확하게하는 것이 최선. 코드 설명에 주석 추가. 아니면 CL 설명
스스로 생각해보기
- 리뷰어의 제안이 최선이 아닐수도 있음.
- 확신은 미뤄두고 잠시 물러서서 생각해보자
의견 충돌 해결하기
- 이견이 있다면 합의를 시도하자
- 합의점이 없다면 코드 리뷰 표준 원칙을 참고하자
리뷰어
1. 코드 리뷰의 표준 원칙
코드 리뷰어는 코드 변경점(changelist)이 완벽하지 않더라도 코드 품질이 확실히 개선되는 상태가 되었다면, 리뷰어는 해당 변경점을 승인하는 방향으로 고려해야 합니다.
여기서의 핵심은 “완벽한” 코드란 것은 없으며 더 나은 코드만 있다는 것입니다. 리뷰어는 리뷰 승인을 하기 전에 코드 작성자에게 변경사항의 모든 작은 부분까지 완벽하게 하도록 요구해서는 안됩니다. 오히려 리뷰어는 변경 사항의 중요점과 앞으로 진행해야 하는 방향을 비교하여 균형을 맞추어야 합니다. 그러니까 완벽을 추구하기보다 지속적인 개선을 리뷰어는 추구해야 합니다. 따라서 완벽하지 않더라도 코드 변경사항이 유지보수, 가독성을 개선시킨다면 며칠 또는 몇 주 동안 지연되어서는 안됩니다.
리뷰어들은 항상 무엇인가 더 나은 방법이 있을 수 있다는 의견을 자유롭게 남길 수 있어야하지만, 그러나 그것이 그다지 중요하지 않다면 "Nit:" 과 같은 접두어를 붙여 코드 작성자가 선택할 수 있도록 해야 합니다. => 멘토링시 활용
주의: 이 문서에서 코드의 품질을 악화시키는 코드 변경사항 승인을 정당화하지 않습니다. 이는 오직 긴급상황일 경우에만 가능합니다.
[역주] Nit은 Nitpick을 뜻합니다. 우리말로 번역하면 "트집 잡다"
- 기술적인 사실과 데이터는 의견과 개인의 선호보다 우선시되어야 합니다.
- 코딩 스타일에 있어서는, 스타일 가이드를 절대적으로 준수해야 합니다. 스타일 가이드에 없는 공백 등은 개인 취향입니다.
- 소프트웨어 설계에 관해서는 절대 스타일이나 개인적인 선호를 따라서는 안됩니다. 코드 작성자가 (데이터를 통해 또는 SOLID 개발 원칙에 근거하여) 여러가지 접근 방법이 유효한 것을 입증한다면, 리뷰어는 코드 작성자의 선호를 받아들여야 합니다. 그렇지 않으면 소프트웨어 설계의 표준 원칙에 따라서 결정됩니다.
- 정한 규칙이 없다면, 리뷰어는 시스템의 전체 코드 품질을 악화시키지 않는 수준에서 기존 코드와 일관성이 있도록 코드 작성자에게 요구할 수 있습니다.
2. 코드 리뷰에서 보아야 하는 것
- 코드가 잘 설계되어있다.
- 기능이 사용자에게 유용하다.
- UI 변경사항이 합리적이고 보기 좋다.
- 병렬 프로그래밍이 안전하게 수행된다.
- 코드가 필요 이상으로 복잡하지 않다.
- 현재 필요없는 기능을 구현하지 않았다.
- 단위 테스트가 적절하다.
- 테스트가 잘 설계되어 있다.
- 개발자가 모든 것에 명확한 작명을 했다.
- 주석은 명확하고 유용하며, 대부분 무엇이 아닌 이유를 설명한다.
- 적절히 문서화되었다.
- 스타일 가이드를 잘 지켰다.
리뷰어는 요청 받은 모든 코드 라인을 리뷰하고, 문맥을 살피고, 코드 품질을 개선하고 있는지, 코드 작성자가 잘한 점에 대해서 칭찬한다.
3. 코드 리뷰 범위 탐색
광범위하게 보고 주요 부문 보기
- 변화가 의미가 있습니까? 좋은 설명이 있습니까?
- 변화의 가장 중요한 부분을 먼저 보세요. 전체적으로 잘 설계되었습니까?
- 나머지 코드 변경사항을 적절한 순서로 보세요.
4. 코드 리뷰의 속도
구글에서는 개인이 코드를 작성할 수 있는 속도를 최적화기보다 팀이 제품을 함께 생산할 수 있는 속도를 최적화합니다. 개인의 개발 속도도 중요하지만 팀 전체의 속도만큼은 중요하지 않습니다.
- 코드 리뷰 요청을 받자마자 진행해야 합니다. 업무일 기준으로 하루를 넘어서는 안됩니다.
- 당신이 코드 작성과 같이 집중된 작업을 하고 있다면, 코드 리뷰를 위해 일을 멈추지 말아야 합니다.
- 코드 리뷰의 속도를 높이기 위해 리뷰어가 몇 개의 미해결 의견을 남기더라도 리뷰를 승인할때도 있다.
- 코드 작성자가 너무 큰 코드 변경사항에 대해 리뷰를 요청하면, 여러 개의 작은 코드 변경 사항으로 나누도록 요청해야 합니다.
- 긴급상황에는 리뷰 프로세스를 빠르게 통과시키거나 코드 품질 규칙을 완화시켜야 하는 경우도 있습니다.
5. 코드 리뷰에 의견 작성하는 방법
- 친절해야 한다.
- 논증하라.(자신의 추론을 입증하라)
- 명확한 가이드를 주는 방법과 문제점만 제시하고 코드 작성자가 결정하는 방법을 균형있게 한다. => 수정할 책임은 작성자에게 있음을 명심하라.
- 코드 작성자에게 복잡한 부분에 대한 설명을 요청하는 대신에 코드를 단순화하거나 주석을 추가하도록 장려하라.
출처 : https://madplay.github.io/post/google-code-review-guide
'Methdology > Agile' 카테고리의 다른 글
성공적인 패어 프로그래밍을 위한 레시피 (0) | 2017.06.07 |
---|