Computer 그리고 Developer/Article-번역

코드 리뷰 가이드라인 - Part 1(Code review guidelines)

버리야 2014. 2. 9. 23:27
반응형



사내에서 코드리뷰 가이드라인에 대한 좋은 글을 공유하여서 번역하여 공유하면 좋을것같아 

발번역을 해본것을 블로그에 공유하여 올린다.

왜 리뷰가 중요한가?(Why Reviews are important?) 부분은 제외한 모든 부분을 번역하였다



Code review guidelines – Part 1

최초작성일 : 2014.2.9

작성자:버리야 날자



코드 리뷰가 무엇인가?

코드 리뷰는 컴퓨터 소스의 체계적인 검사이다(peer review로 알려져있다). 
코드 리뷰는 초기 개발단계에서 간과한 실수를 찾고 수정하고, 전체 소프트웨어의 품질과 개발자의 기술 모두를 개선하기 위한것이다.

왜 리뷰가 중요한가?

(..생략..)

코드리뷰의 주요 목표


1. 개발 진행중에 일찍 결함을 발견하고 고치기 위해

2. 팀 멤버로써 코드 베이스의 이해를 공유를 통해 서로 배울수 있다.

3. 디자인과 구현에서 일관성의 레벨을 유지하기 위해 도움을 준다.

4. 재작업을 줄이는 것과 같이 팀 전체에 흔한(common) 결함을 확인하기 위해 도움을 준다.

5. 실행의 기술적인 퀄리티에 대해 이해관계자(stakeholders)의 신뢰를 쌓는다.

6. 이해의 균등은 팀 멤버들중의 한명이 일을 할 수 없을 경우에 팀 멤버들의 교대(interchangeability)에 도움을 준다.

7. 다른 관점을 갖는다. 코딩과 테스팅 팀과 분리한 이유와 비슷하게, 동료의 리뷰는 문제를 발견하기 위해 필요한 거리를 제공한다.

8. 자부심/보상. 코딩 기량의 인정은 많은 프로그래머들을 위해 중요한 보상이다.

9. 팀의 응집력. 일을 같이 하는것은 팀 멤버들간에 더 가까워질 수 있게 도움을 준다. 또한 코딩이 가져오는 고립에서 잠시동안 벗어나게 해준다.


아래 그래프에서 개발 과정에서 잠재적인 결함을 고치기 위한것이 얼마나 중요한지를 보여준다.





리뷰어가 초점을 맞추어야 할 주요 부분은 다음과 같다.

  • 일반적인 Unit Testing
  • 코멘트와 코딩 컨벤션
  • 에러 처리
  • 빠진 리소스가 있는지
  • Thread Safety
  • Control Structures
  • 성능
  • 기능성(Functionality)
  • 보안
  • 존재하는 디자인과 아키텍쳐의 준수
  • 3rd party 라이브러리의 사용이 적절한지


역할과 책임

1. 개발자 : 리뷰될 코드를 작성하고 리뷰 요청을 한 사람.

2. 리뷰어(들) : 코드를 리뷰하고 개발자에게 찾아낸것을 보고(report)할 사람들.



개발자를 위한 팁 : 

1. 주요 평가자는 저자(코드를 작성한 사람) 즉 당신이다.


2. 코드 리뷰시 초점을 맞출만한 것을 위주로 자신만의 체크리스트를 만든다.
이 체크리스트 중의 일부는 같이 넣어 쉽게 해야한다. 코딩 표준 문서의 규격을 따라야 한다. 당신의 체크 리스트 이기 때문에, 평소 자주 문제가 되던 것에 중점을 두고, 그렇지 않은 것은 넘어 갈 수 있다. 만일에라도 문제가 발생한다면, 체크 리스트를 활용하여 당신의 코드 상의 문제를 수정할 수 있다. 당신의 팀이 찾아낼 것들에 대한 것을 줄일 수 있을 뿐만 아니라, 코드 리뷰 회의를 빨리 끝낼 수 있을 것이다. - 그리고 리뷰하는 시간이 줄어 모두가 기뻐할 것이다.

3. 당신은 당신의 코드가 아니다. 
리뷰의 주목적은 문제를 찾아내는 것이라는 것을 기억해야 한다, 그리고 문제들은 찾아 내질 것이다. 누군가가 찾아 낸 것에 대해서 마음에 담아 두지 말 것.

4. 자신이 실수를 할 것이라는 것에 대해 이해하고, 받아들 일 것. 
제품으로 만들기 전에 실수를 먼저 찾아내는 것이 주요점이다. 운좋게도, JPL의 로켓 가이드 소프트웨어를 개발하는 일부를 제외하고는, 우리 산업에서 실수들은 거의 치명적이지 않다, 그래서 우리는 할수 있고, 해야하며, 배우고, 웃고, 넘겨야 한다.

5. 당신이 얼마나 “karate(손발을 이용해서 싸우는 일본 권법)”에 대해 아는지는 몰라도, 누군가는 항상 더 많이 알고 있다. 
개개인은 당신에게 새로운 길을 알려줄 수 있다. 다른이들에게서 찾아내고, 받아들여라, 특히 자신이 더 배울게 없다고 생각할 때.

6. 상의 없이 코드를 재작성하지 마라. 
“코드 수정" 과 “코드 재작성" 사이에는 미묘한 차이가 있다. 차이를 알고, 외로운 집행자(lone enforcer)로써가 아닌 코드리뷰의 프레임워크안에서 문체(양식)의 변화를 추구해아한다.

7. 세상에 하나 뿐인 상수는 변화이다. 
웃음으로 변화를 준비하고, 받아 들일 것. 각각의 요구조건들, 플랫폼 또는 툴을 싸워야 할 심각한 것으로 여기지 말고, 새로운 도전으로 바꾸어 볼 것.

8. 당신이 믿는 것을 위해 싸우되, 패배를 정중히 받아 들일 것. 
당신의 아이디어가 무시될 수도 있다는 것을 이해하라. 결과적으로 당신이 옳았어도, 복수 하려하거나, “내가 그렇다고 했잖아" 라고 여러번 말하지 말 것. 그리고, 당신의 비싼 대가를 치르고 세상을 떠난 아이디어 희생이나 슬로건을 만들지말라.

9. 방에만 있는 사람이 되지 마라. 
어두운 사무실에서 콜라를 사러 갈 때만 일어나는 사람이 되지 마라. 방에만 있는 사람은 접촉이 없고, 보이지 않고, 통제불가능이고 열린공간이 없고 협업환경이 없다.

10. 리뷰 회의는 문제해결 회의가 아니라는 것을 반드시 알릴 것.


11. 코딩 표준을 유지하도록 도울 것. 
코딩 표준에 등록 되지 않은 논의 되었던 것들을 코딩 표준에 등록하도록 제안하라. 과제 중 하나는 투쟁 적인 코드 리뷰 습관을 가진 조직의 개발자들은 주로 어디서 다음 문제가 생길지 모른다는 것이다. 만약 당신이 각 이슈를 코딩 표준들에 문서화 한다면, 다음번 당신의 코드 리뷰때 당신의 체크리스트와 비교해 볼 수 있다. 이것은 또한 당신의 마음에 컨셉을 확고히 하는데 도움이 될것이고 피드백을 얻기 위한 기회를 놓칠 가능성이 적을것이다.


리뷰어들을 위한 팁

1. 사람 대신에 코드를 비평하라. 개발자에겐 친절하지만 코드에 대해선 불친절하라.

가능한한 당신의 모든 코멘트를 긍정적으로 작성하고 코드를 개선시키기 위해 지향하라. 로컬 표준(local standards), 프로그램 스펙, 성능향상등에 관련한 코멘트.

2. 당신보다 더 모르는 사람을 존경, 존중, 인내심으로 사람들을 대하라. 
주로 개발자를 상대하는 대다수의 비개발자는 우리는 prima donnas(자기가 아주 중요한 인물인 줄 아는 사람)가 최고이고 crybabies(울보)가 최악이다라는 일반적인 의견을 가지고 있다. 분노와 조바심으로 고정관념을 강화하지 말라.

3. 직위(position)로부터가 아닌 지식으로부터의 진정한 권위. 지식은 권위를 불러일으키고, 권위는 존경심을 불러일으킨다. - 만약 당신이 객관적인 환경에서 존경받길 원한다면 지식을 쌓아라.

4. 리뷰 회의는 문제를 해결하기 위한 회의가 아님을 주의하라.


5. 문장보다는 질문을 하라.
문장은 비난을 하는것이다. “당신은 여기서 표준을 지키지 않았어요.”는 의도적이든 아니든 공격이다. “당신이 사용한 접근방법의 이유가 무엇인가요?”이란 질문은 정보를 더 찾고 있다. 분명히, 이 질문은 빈정되거나 잘난체하는 어투로 말하면 안된다. 
하지만 정확하게 질문을 했다면, 이것은 개발자가 그들의 생각을 이야기하고 만약 더 좋은 방법이 있는지 물을 수도 있다.

6. “Why”라는 질문은 피하라.
때로는 극도로 어렵긴 하지만, “Why” 질문을 피하는것은 분위기를 상당히 개선할 수 있다. 대부분의 “Why” 질문은 “why”라는 단어를 포함하지 않은 질문으로 바꾸어 사용할 수 있고 그 결과는 인상적일 수 있다. 
예를 들어, “왜 당신은 여기서 표준을 따르지 않았나요..” 와 “여기에서 표준을 벗어난 이유는 무엇이 있을까요?”의 질문 방법.

7. 칭찬하는 것을 잊지말라.
코드 리뷰의 목적은 개발자들에게 그들이 얼마나 개선할 수 있는지에 초점을 맞추는 것이 아니고, 그들이 한 잘한일이 필연적이 아니다. 인간의 본성은 우리의 성공을 위해 우리가 원하고 필요하도록 인정받는것이지, 우리의 잘못함을 보여주고 싶지 않은 것이다. 
왜나하면 개발은 필연적으로 개발자가 그들의 영혼을 불어넣는 창조적인 일이고, 이는 종종 그들의 가슴속 가까이에 있을 수 있다. 그래서 비난보다 칭찬이 더 필요하다.

8. 당신은 당신이 참조하는 좋은 코딩 표준을 가지고 있다는 것을 명심하라.
코드 리뷰는 조직의 코딩 표준에서 그들의 토대를 찾는다. 코딩 표준은 개발자들이 서로가 품질 유지가능한 코드를 작성하기 위해 공유된 계약으로 가정된다. 만약 당신이 당신의 코딩 표준에 없는 항목을 논의하는 경우 코딩 표준에서 항목을 얻기 위해 할 일이 있다. 
당신은 당신의 코딩 표준에 논의될 항목인지 아닌지 당신 스스로에게 자주 질문을 해야 한다.

9. 해결책에 접근하기 위한 한가지 이상의 길이 있다는 것을 기억하라.
개발자는 당신이 얼마나 다르게 무언가를 코딩했을 수도 있지만 그것이 반드시 잘못되지는 않는다. 목표는 품질과 지속가능한 코드이다. 그것이 이러한 목표를 충족하고 코딩 표준을 따른다면, 그것이 당신이 질문할 수 있는 전부이다.

10. 당신은 코드리뷰를 서두르지 않아도 된다. 하지만 바로 해야할 필요가 있다.
당신의 동료들은 당신을 위해 기다리고 있다.

11. 한번에 200-400 라인보다 적은 코드를 리뷰하라.


보안 코드 리뷰
만약 어플리케이션이 보안에 매우 주의를 필요로 하는 경우 www.owasp.org 을 참고하라.
매우 좋은 보안 코드 리뷰 문서  https://www.owasp.org/images/2/2e/OWASP_Code_Review_Guide-V1_1.pdf

결과를 리뷰하기 위한 심각도 할당.


1. 이름 컨벤션과 코딩 스타일 = Low
2. Control Structures와 Logical 이슈 = Meduim or High
3. 불필요한 코드 = High
4. 성능 이슈 = High
5. 보안 이슈 = High
6. 확장성 이슈 = High
7.  기능적인 이슈 = High
8. 에러 처리 = High
9. 재사용 가능성 = Medium
10. 디자인 & 아키텍쳐 = High
11. 3rd party 라이브러리의 적절한 사용 = Medium


개발자와 리뷰어들을 위한 체크리스트


개발자를 위한 체크리스트


코드가 컴파일 되었나

개발자 테스트 및 유닛 테스트 포함 여부

javadoc이 제 위치에 있는가

코드가 깔끔한가 (indentation, 라인 길이, no commented-out code , 틀린 철자, etc)

적절한 예외 처리를 고려하였나

로깅을 적절하게 사용하도록 만들었나

사용하지 않는 import들을 제거 하였나

이클립스의 경고들을 모두 처리 하였는가

가능한 NPEs를 고려하였는가

코딩 표준을 따르는가

코드에 불필요한 것이나 테스트 루틴들이 남아 있는가

하드코딩된 것이나 개발시에만 필요한 코드가 여전히 남아있는가

성능에 대한 고려가 되었나

보안에 대한 고려가 되었나

자원 해제에 관한 코드가 있는가(HTTP connections, DB connection, files, etc)

문서화가 잘되어있는지 아니면 프레임워크의 알려진 제한 사항에 대한 해결방법이 있는지

외부 재활용 컴퍼넌트 또는 라이브러리 함수들에 콜에 의해 바뀔 수 있는 코드가 있는가

Thread safety한지 deadlock 가능성은 없는지

코드가 해야 할 일을 적절히 하고 있는가

내가 변경 했을 때 현재 기능에 side effect를 줄만한 것이 없는가 

코드가 현재 디자인/아키텍쳐와 일치하는가

새로운 것을 구현하기 전에 기존의 메소드/유틸들을 확인


리뷰어들을 위한 체크리스트


코드가 비즈니스 요구사항을 충족하는가

코멘트가 이해할수 있고 코드를 유지할 수 있는 것을 추가하였는가

코멘트는 너무 많아도 안되고 장황해도 안된다.

타입이 가능한곳에 일반화되어 있는가

매개변수 타입이 적절하게 사용되었는가

Exception이 적절하게 사용되었는가

반복적인 코드가 뽑아내어져 있는가

프레임워크가 적절하게 사용되어있는가 - 메소드가 모두 적절하게 정의되어 있는가

Command 클래스가 단 하나의 작업을 수행하도록 설계되어 있는가

JSP가 비즈니스 로직을 포함하고 있지 않는가

Unit test는 현재 코드로 작성되어있는가 그리고 정확한가

흔히 있는 에러를 체크하라

잠재적인 스레딩 이슈가 있을만한 곳에 제거되어있는가

모든 보안 문제가 해결되어있는가

성능이 고려되어있는가

이 기능은 현재 디자인과 아키텍쳐에 맞는가

코드가 유닛테스트 가능한가

코드가 이치에 맞지 않은 static methods/block을 사용하지 않는가

코드가 코딩 표준을 준수하는가

Logging을 적절히 사용하였는가(적절한 logging 레벨과 세부사항)

NPEs and AIOBs

(이미 있는것을 다시 만드느라) 쓸데없이 시간을 낭비한 코드가 아닌가

코드가 기존 기능의 side effect가 없는가



또한 프린트할 수 있게 checklist format으로 만들어두었다.









반응형
저작자표시 비영리 변경금지

'Computer 그리고 Developer/Article-번역'의 다른글

  • 현재글코드 리뷰 가이드라인 - Part 1(Code review guidelines)
댓글

티스토리툴바