Pull Request 검토 지침

다른 사람의 코드를 검토하는 일은 Bahmni(또는 어떤 오픈 소스 코드)에든 기여할 수 있는 가장 가치 있는 활동 중 하나입니다. 의미 있는 코드 리뷰에 기여하기 위해 선임 개발자나 핵심 팀원일 필요는 없으며, 리뷰를 한다고 해서 반드시 직접 코드를 병합해야 하는 것도 아닙니다. 일부만 검토해도 최종 검토와 병합 전에 기여자가 PR을 반복해서 개선하는 데 도움이 됩니다.

요청자에게 방향을 제시하는 데 도움이 되는 PR 지침 페이지를 참고하세요.관련 문서: Pull Request 제출

1. 기능/문제 설명과 인수 기준이 명확히 적힌 JIRA 카드가 생성되어 있습니까? PR에 올바른 JIRA 카드 번호가 표시되어 있습니까? 병합할 때는 커밋 메시지에 JIRA 카드 번호가 반영되었는지 반드시 확인해야 합니다. 이 PR을 검토하기에 적합한 사람입니까? 아니라면 적합한 사람에게 연결해 주세요. 일부 맥락만 알고 있다면 더 잘 아는 사람에게 도움을 구하세요. travis-ci, codeclimate 등의 검사가 실패했습니까? 그렇다면 기여자에게 즉시 확인을 요청하세요. 제출자가 CLA에 서명했습니까? cla-assistant 검사 상태를 확인하세요. 아니라면 정중히 서명을 요청하세요. 단정적으로 말하기보다 질문하세요. 비난으로 들리기 쉬운 ‘왜’를 피하고, ‘그렇게 한 이유는 무엇인가요?’처럼 맥락과 판단 근거를 이해하려고 하세요. 큰 PR은 작성자와 직접 만나 함께 검토하는 것이 가장 효과적일 수 있습니다. 큰 PR은 별도 브랜치에 병합해 로컬에서 실행·확인한 뒤 커밋하는 편이 좋습니다. GitHub의 ‘Command line instructions’ 안내에 따라 PR을 로컬에 병합한 뒤 master로 푸시할 수 있습니다. 코드의 정확성을 로컬 실행 없이는 확신하기 어려운 큰 PR에서는 이 방식을 권장합니다. GitHub의 PR 링크로 이동해 하단 ‘merge pull request’ 버튼 옆의 ‘command line instructions’를 클릭하고 안내를 따르세요.

1. 코딩 스타일은 중요하지만 지나치게 강요해서는 안 됩니다. 업그레이드할 때 기존 기능이 깨지면 안 됩니다. 데이터 마이그레이션이 필요하다면 기존 트랜잭션 데이터를 보존하고 계속 사용할 수 있도록 해야 합니다. 새 설정을 도입할 때는 이전 동작을 기본값으로 유지하고, 동작을 확장하거나 변경하는 선택지를 제공해야 합니다. 즉 기존 설정에 새 코드를 적용할 때 각 구현에서 별도 업데이트가 필요하지 않아야 합니다. 설정은 문서화하고 적용되는 릴리스 버전을 명시해야 합니다. 기능은 가능한 한 이전과 비슷하게 동작하고 쉽게 익힐 수 있어야 하며, 가급적 최종 사용자 재교육이 필요하지 않아야 합니다.

1. 사용하지 않는 코드를 남겨 두지 마세요. 주석 처리된 코드도 쓸모없는 짐이 되어 혼란을 일으키기 쉽습니다. 어차피 Git을 사용하며 모든 편집기에서 버전 간 차이를 잘 보여 줍니다. 중복 코드가 있다면 별도 카드를 만드는 것을 고려하세요.

1. try-with-resources에서 Closeable 사용, 스트림 사용, ‘names.forEach’ 같은 반복문, 람다 표현식을 검토하세요.

1. 코드 리뷰는 ‘지시’나 ‘명령’이 아니라 토론입니다. 정확성, 설계, 보안, 가독성, 유지보수성에 대해 검토자도 기여자와 동등한 책임을 집니다. 검토자가 만족하지 못했다면 병합하지 말고 자유롭게 이의를 제기하세요. 기여자가 논리적인 설명을 제시한다면 검토자의 의견에 동의하지 않아도 괜찮습니다. 따라야 할 예시로 관련 코드 구간을 기여자에게 안내하세요. 커밋이 여러 개여서 검토하거나 로컬에서 테스트하기 어렵습니까? 기여자에게 하나로 squash하도록 요청할 수 있지만, GitHub에서 ‘squash and merge’를 제공하므로 일반적으로 따로 요청할 필요는 없습니다. PR 관련 코드에 대응하는 테스트가 있는지 확인하세요. 단위 테스트와 해당되는 경우 통합 테스트가 있어야 합니다.

1. JIRA 카드에 ‘needs-doc-update’ 레이블을 추가해 표시하세요. 기여자에게 문서 추가를 요청하고 ‘버전 X부터 적용’ 같은 적절한 정보가 포함되었는지 확인하세요.

1. 라이브러리 의존성을 추가한다면 잘 알려져 있고 지원되는 최신 라이브러리인지, 목적에 적합한지, 기능이 중복되지 않는지 확인하세요(예: Java 8 날짜 API로 해결할 수 있다면 Joda datetime 라이브러리를 추가하지 않음). 다른 모듈에 의존한다면 해당 모듈이 릴리스되었는지 확인하세요. 스냅샷 릴리스 의존성은 권장하지 않습니다. 예외적으로 허용할 수 있지만 그 의존성을 해소하기 위한 JIRA 카드가 있는지 확인하세요. 당연히 컴포넌트·서비스·코드 사이에 순환 의존성이 있어서는 안 됩니다.

다음 릴리스에서 코드 리뷰가 필요한 카드는 JIRA에서 확인하세요.

https://bahmni.atlassian.net/issues/?jql=status+%3D+%22Pull+Request+%2F+Code+Review%22+AND+fixVersion+%3D+earliestUnreleasedVersion%28%29+ORDER+BY+rank+