이 글은 지난 1차 OT 이후 글 이후의 과정을 담았습니다.
지난 1차 OT에서 각 파트의 프로젝트 요구사항으로 지정했던 것은 코드 컨벤션과 Git 전략에 대한 서술이었습니다.
코드 컨벤션 : 코드 작성 시 팀원끼리 지켜야할 합의된 규칙을 의미합니다.
Git 브랜칭 전략 : 브랜치 생성 / 삭제 / 병합에 대한 팀원간의 합의된 규칙을 의미합니다.
이번주 진행사항은 파트끼리 합의한 하나의 규칙을 선정하는 것이었습니다.
따라서, 1차적으로 제가 진행했던 프로젝트 `오디`의 백엔드 컨벤션을 기반으로 각 팀원이 추가하고픈 내용 / 삭제하고픈 내용을 자율적으로 적어와 회의가 진행되도록 하였습니다.
그럼 컨벤션 회의에서는 어떤 내용들이 오고갔을까요?
코드 컨벤션
백엔드 코드 컨벤션은 크게 3가지로 나누어졌습니다.
1) 코드 스타일
2) 테스트 코드
3) 코드 리뷰
1) 코드 스타일
모든 컨벤션에 대해 이야기하기보다 서로 논의하여 구체화되었던 3가지 룰을 중심으로 소개해보겠습니다.
1-1) public 메서드와 관련있는 private 메서드의 정렬을 어떻게 하는가?
public 메서드와 관련있는 private 메서드의 정렬 규칙에 대한 구체화입니다.
public void A() {
a();
}
private void a() {}
public void B() {}
예를 들어 public 메서드 A와 B가 있고 A에서 private 메서드 a가 사용중이라면 로직을 한번에 보기 위해 관련있는 public 메서드인 A 아래에 위치시킨다는 룰이었습니다.
그러나, 팀원인 비토는 여러 public 메서드에서 사용하는 private 메서드의 정렬순에 대해 이야기하였습니다. 예를 들어 앞선 예시에서 a()라는 private 메서드가 A와 B 둘다 쓰이면 어디에 위치해야 하는가? 에 대한 물음이었습니다.
public void A() {
a();
}
public void B() {
a();
}
private void a() {}
팀원들과 논의한 결과 여러 public 메서드에서 사용하는 private 메서드는 맨 아래 위치시키는 것으로 합의되었습니다.
1-2) 정적 임포트를 허용여부 구체화
Enum Type이나 다른 클래스의 정적 필드를 임포트할 때 static 임포트를 허용하는가? 에 대한 부분입니다.
ex) 허용 : Category.FOOD / 비허용 : FOOD
오라클 문서에 따르면 정적 임포트를 매우 아껴서 사용(Very Sparingly)하는 것을 권장하고 있습니다. 그 이유는 정적 임포트를 하면 권한이 부여되지 않은 멤버들까지 모두 임포트되기 때문입니다. 예를 들어 Math.abs를 Math.*로 정적 임포트하여 사용하면 abs 뿐만 아니라 Math에 관한 다른 정적 필드와 메서드들 또한 임포트가 됩니다.
따라서 정적 임포트를 허용하지 않는 것을 룰로 정하였는데요. 그러나 AssertJ, Junit, Mockito, RestDocs와 같은 테스트 관련 임포트의 경우는 정적 임포트를 허용해야 한다는 팀원들의 의견이 있었습니다.
그 이유는 다음과 같습니다.
- 비즈니스 코드는 임포트를 통한 결합도 관리가 중요하나 테스트는 상대적으로 그 중요도가 덜하다.
- 테스트 코드가 사용하는 정적 메서드는 반복적이다.
- 테스트 코드가 사용하는 정적 메서드는 가독성이 중요하다.
따라서 다음과 같이 컨벤션을 수정하였습니다.
1) 비즈니스 코드에서의 정적 임포트는 지양한다.
2) 테스트 코드에서의 정적 임포트는 부분적으로 허용한다. ex) AsserJ, Junit, Mockito, RestDocs
1-3) 정적 팩토리 메서드 금지
팀원간에 가장 많은 의견 충돌이 있었던 부분입니다.
이펙티브 자바 1단원 1장에는 `생성자 대신 정적 팩터리 메서드를 고려하라` 라는 단원이 있습니다.
그리고, 정적 팩터리 메서드의 장점을 나열하는데요.
1) 의미를 담을 수 있다.
2) 하위 객체를 반환할 수 있다. (상위 객체를 반환형으로 지정하고, 서브 타입 객체를 반환할 수 있으므로)
3) 꼭 새로운 객체를 생성하지 않아도 된다. ex) 싱글톤 패턴
4) 생성로직을 캡슐화할 수 있다.
문제는 프로젝트를 하면서 주로 정적 팩토리 메서드를 사용하는 맥락이 1번에 치중되어 있는 경우나, 반복되는 생성로직을 객체에게 넘겨주기 위한 경우가 많았고 그 판단의 기준이 팀원마다 애매했다는 것입니다.
이에 비토는 정적 팩토리 메서드 강경 반대파로서 팩토리 메서드의 장점을 명확히 느낀 순간이 적기에 사용 반대 룰을 가결시키려 노력했던 반면, 또 다른 팀원인 커찬은 본인 프로젝트의 예시를 기반으로 정적 팩터리 메서드의 장점도 존재함을 설득했습니다.
public static Member createMaster(String nickname, Room room) {
return new Member(nickname, room, true);
}
public static Member createCommon(String nickname, Room room) {
return new Member(nickname, room, false);
}
커찬이 제시했던 예시는 방장(Master)인 경우와 참여자(Common) 멤버를 생성하는 반복로직의 경우, 정적 팩토리 메서드가 충분한 의미를 담을 수 있으며 반복적으로 입력해야 하는 boolean 값을 캡슐화하여 휴먼에러를 방지할 수 있음을 어필했습니다.
그러나, 우리 프로젝트에서 다음과 같은 상황이 있으리라 확신할 수 없었기에 우선적으로 정적 팩터리 메서드를 금지하고 추후 프로젝트 개발 상황에 맞추어 재고해보자는데 의견이 맞추어졌습니다.
이에 따라 확정된 컨벤션은 [정적 팩터리 메서드는 Record형 DTO에만 허용한다] 입니다.
2) 테스트 코드
테스트 코드에 대한 컨벤션은 지난 프로젝트 컨벤션에 비해 큰 변화가 없었습니다.
- 테스트 코드의 설명은 @Displayname을 사용하고, 메서드명은 테스트하는 메서드명을 그대로 사용한다.
- 상위 Nested Class 명은 메서드로 한다
- 하위 메서드 테스트명은 한글로 적는다.
- 예시
-
class RoomTest { @Nested class CreateNewRoom { @Test void 게임이_준비_상태일_떄_게임을_시작할_수_있다() { Room room = Room.createNewRoom(); room.startGame(); assertThat(room.isGameProgress()).isTrue(); } @ParameterizedTest @EnumSource(mode = Mode.EXCLUDE, names = {"READY"}) void 게임이_이미_시작했다면_예외를_던진다(RoomStatus status) { RoomSetting roomSetting = new RoomSetting(5, 10_000, Category.IF); Room room = new Room("uuid", 1, status, roomSetting); assertThatThrownBy(room::startGame) .isExactlyInstanceOf(NotReadyRoomException.class); } } }
- given-when-then 주석은 달지 않고, 개행한다.
- 더미 데이터를 추가하지 않고, 각 테스트 케이스마다 작성한다.
- 각 layer의 테스트 환경을 BaseXXXTest로 설정하고 상속받아 테스트 클래스를 작성한다.
단위 테스트
- Domain
- Repository - @DataJpaTest 를 사용한 테스트
- Service - @SpringBootTest(webenvirent=NONE) 을 사용한 테스트(h2)
- Controller - @SpringBootTest(webenvirent=RANDOM_PORT) 을 사용한 테스트
- UI를 제외한 모든 코드는 테스트한다 기준으로 테스트한다.
- JPA를 사용하는 Repository 기본 제공 CRUD에 대한 테스트는 제외한다.
- 단, 단순 전달 메서드는 테스트하지 않는다.
한 가지 이번 프로젝트를 통해 새로 시도하는 부분은 @Nested를 통해 각 method에 따라 테스트를 분리하고 한글로 테스트명을 작성한다는 점입니다. 가독성/테스트 목적의 직관적인 파악을 위해 커찬이 제안한 룰입니다.
3) 코드 리뷰
- PR merge 기준 : 전원 approve
- 코드 리뷰의 기한
- PR제출일 (월-목) : 24시간 이내
- PR제출일 (금-일) : 48시간 이내
- 확인했음은 👍 이모티콘으로 통일한다.
- 코멘트는 [제안] - [질문] / [필수] 2단계로 답글을 남긴다.
- 코드 리뷰를 남기거나 반영했을 때 /noti 웹훅으로 리뷰 여부를 남긴다.
여기서 추가 설명이 필요한 것은 /noti 룰인데요.
비토가 Github Actions로 /noti를 포함한 comment를 남길 시 Discord 웹훅으로 comment를 자동 발송하는 action을 설정하여 쉽게 리뷰 반영 및 완료 여부를 알 수 있게 되었습니다.
Git 브랜칭 전략
먼저 각 팀원들이 속했던 프로젝트에서는 어떤 git 브랜칭 전략을 사용했는지, 그리고 어떤 이슈 템플릿이나 PR 템플릿을 사용했는지 파악하여 비교분석해보았습니다.
먼저 커찬이 속해있던 땅콩팀의 컨벤션 및 Git 브랜칭 전략입니다.
Git
- git branch 전략
- feat/ refactor / test + #번호 로 구분
PR
- CheckList를 통해 PR을 보낼 때 잊지 말아야 하는 부분을 체킹
- AS-IS / TO-BE로 PR의 목적을 드러냄
Issue Number
As-Is
To-Be
Check List
-[] 테스트가 전부 통과되었나요?
-[] 모든 commit이 push 되었나요?
-[] merge할 branch를 확인했나요?
-[] Assignee를 지정했나요?
-[] Label을 지정했나요?
Test Screenshot
(Optional) Additional Description
Issue
- [FEAT], [REFACTOR], [FIX] 등 목적 구분
- Assignees, Label, Projects 설정
- ex) [FEAT] google analytics 고도화하여 "땅콩" 서비스에 필요한 데이터 수집
### Description 💭
### TODO ✅
---
다음으로 비토가 속한 포케로그 팀의 컨벤션 및 Git 브랜칭 전략입니다.
Git
- git branching 전략
- be/feat/#이슈번호 - 이슈 내용
- ex) be/feat/#463-change-cloud-front
PR
🍄 PR 확인 사항
PR이 다음 요구 사항을 충족하는지 확인하세요. :
- [x] API 명세서가 업데이트 혹은 작성이 되어 있나요?
현재 작업은 어떤 이슈를 해결한 것인지 설명해주세요.
Issue Number: #이슈번호
- [x] 검증 테스트 작성
- [x] 테스트 전부 정상화
기존 코드에서 변경된 점이 있다면 설명해주세요. (추가 X)
Issue
- 헤더 : [BE-INFRA] / [BE-FEAT]
- Assignees와 Label만 신경 씀
### Description 💭
### TODO ✅
---
- 특징적으로 브랜치 네임에 이슈 내용을 포함하는 것이 인상적이었습니다.
Git 컨벤션 마이그레이션하기
회의가 시작되고 비토와 커찬에게 각 팀의 Git 컨벤션과 템플릿을 공유하였습니다. 그리고 공유된 맥락을 기반으로 서로가 팀에서 효용을 보았던 부분들, 쓸모없던 부분들을 이야기하며 팀의 Git 전략을 확정하였습니다.
Debate Timer Git 컨벤션
Git
- 두 팀과 동일하게 제가 속했던 프로젝트도 Git 전략을 사용했기에 Git 브랜칭 전략을 채택하였습니다.
PR
- Assignees와 Label 할당은 Github Actions로 자동화하도록 하였습니다.
- 부가적인 항목들이 크게 효용이 없었다는 공통 의견을 반영하여 이슈와 Description 두 가지로 가장 간단한 템플릿을 채택하였습니다.
#🚩연관 이슈
closed #
# 🗣️ 리뷰 요구사항 (선택)
Issue
- 커밋과 구분되도록 [FEAT] / [CHORE] 등의 헤더를 적도록 하였습니다.
- Description과 Time 만 적도록 Simple 하게 유지하였습니다.
### ✨ Description
### ✨ Time(마감기한)
디베이트 타이머의 코드 & Git 컨벤션 설정에 대한 이야기는 여기까지입니다.
다음 글은 디베이트 타이머의 1차 기능인 [토론 테이블 만들기 와이어 프레임 작성기]로 돌아오도록 하겠습니다.
그럼 오늘도 행복하세요:)
'프로젝트 > 디베이트 타이머' 카테고리의 다른 글
| [디베이트 타이머 - 1차 스프린트] 팀 빌딩과 프로젝트 OT (7) | 2024.12.03 |
|---|
