Skip to content

PR 리뷰 컨닝페이퍼

박태영 edited this page Aug 24, 2024 · 4 revisions

PR 리뷰 컨닝페이퍼

1. 🥳🎉 git, pull request 관련

PR 제출 샘플이 궁금하다면

PR 제출하고 main이 변경되거나, commit이 너무 많아진다면

  • PR 제출 후 main 브랜치와 충돌이 발생한다면 rebase나 merge로 해결하면 됩니다
  • 커밋이 많아져도 직접 squash할 필요는 없습니다
  • 머지할 때 squash할 예정이므로 걱정하지 않아도 됩니다

2. 🔍🦖 코드 컨벤션 관련

주석은 이렇게 달아주세요

  • #region ... #endregion은 피하고 줄바꿈과 // 주석으로 구조를 표현하세요.

변수명에 복수형 표시해주세요

  • 복수형을 나타내기 위해 명확하게 Fixtures, Tests, LogProbs와 같은 변수명을 사용하세요.

record 명은 카멜케이스로 🐫

  • LogProb처럼 record 명은 카멜케이스를 따릅니다.

클래스 정의시 클래스와 속성에 /// 각주를 달아주세요

  • 클래스 정의시 /// 각주로 summary 등 각주를 달아주세요.

DateTime 보다 DateTimeOffset을 추천

  • 날짜+시간 데이터를 다룰 때는 DataTime 보다 DateTimeOffset을 사용하세요.
  • UTC Offset을 포함하므로, 서로 다른 시간대의 값을 비교할 때 유용합니다.

System.Text.Json 참고해서 속성마다 데코레이터로 필수여부를 달아주세요

  • 속성의 필수 여부를 명확히 하기 위해 데코레이터를 사용해주세요.
  • 대신 required 제한자를 사용할 수 있는데 이 경우 컴파일러에서 생성자 초기화를 강제합니다.
// src/AzureOpenAIProxy.ApiApp/Models/AdminEventDetails.cs
    /// <summary>
    /// Gets or sets the event id.
    /// </summary>
    public required string? EventId { get; set; }
  • 위와 같이 JsonProperty 데코레이터 사용할 때는 생성자 초기화를 강제하지 않습니다.
// src/AzureOpenAIProxy.ApiApp/Models/AdminEventDetails.cs
    /// <summary>
    /// Gets or sets the event id.
    /// </summary>
		[JsonRequired]
    public string? EventId { get; set; }

객체의 모든 속성을 nullable로 정의합니다?

  • 객체 모든 속성에 nullable 타입을 적용하세요?
  • 타입 뒤에 ?를 붙이면 됩니다?

테스트에서는 foreach 루프 대신 개별 값을 테스트합니다.

  • foreach 루프 대신 개별 테스트로 값을 검증해주세요.
  • 각 값에 대해 명확하고 독립적인 테스트를 작성합니다.

swagger json을 직접 읽어들여서 테스트할 수 있습니다.

  • 다만 테스트할 때는 단위테스트 취지를 고려하여, 테스트 항목을 나누어 진행해봅니다.
  • 이를테면 이렇게 테스트를 나누어 진행합니다.
    1. AdminEventDetails가 존재하면서 그게 Object인가
    2. 특정 propName이 존재하면서 그게 Object인가
    3. 특정 propNamerequired인가
    4. 특정 propName의 type이 무엇인가

패키지 버전은 메이저 버전만 1.46.* 대신 1.*처럼 다룹니다.

  • 패키지 버전은 메이저 버전만 명기하여 관리합니다.

조회성 엔드포인트에서 200, 404 응답코드를 구분하기

  • 요청받은 리소스가 아예 존재하지 않아 null exception이 발생하면 404
  • 존재하는데 개수가 하나도 없는 빈 컬렉션은 200OK으로 반환