Skip to content

Comments

[육선우] sprint4#77

Open
ryuk6238-dot wants to merge 20 commits intocodeit-bootcamp-spring:육선우from
ryuk6238-dot:육선우sprint4

Hidden character warning

The head ref may contain hidden characters: "\uc721\uc120\uc6b0sprint4"
Open

[육선우] sprint4#77
ryuk6238-dot wants to merge 20 commits intocodeit-bootcamp-spring:육선우from
ryuk6238-dot:육선우sprint4

Conversation

@ryuk6238-dot
Copy link
Collaborator

요구사항

기본

  • 기본 항목 1
  • 기본 항목 2

심화

  • 심화 항목 1
  • 심화 항목 2

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@ryuk6238-dot ryuk6238-dot added the 순한맛🐑 마음이 많이 여립니다.. label Feb 15, 2026
@joonfluence
Copy link

커밋 메세지에 대한 규칙을 아래 컨벤션을 참고하여 작성해주시길 바랍니다
참고 https://www.conventionalcommits.org/ko/v1.0.0/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.gitignore에 추가되어야 할 파일들이 .idea 만 있을까요? 고민해보고 추가해보세요~


@RestController
@RequiredArgsConstructor
@RequestMapping("/api/binarycontents")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v1 버저닝을 추가 해주는 것도 좋습니다 (계속 버전이 변화될 수 있기 때문에) 나중에 추가하긴 어렵거든요

Comment on lines +20 to +36
@PostMapping(
path = "/findAll",
consumes = {MediaType.MULTIPART_FORM_DATA_VALUE}
)
public ResponseEntity<List<BinaryContent>> findAllByIds(
@RequestPart("ids") List<UUID> ids // 파트로 UUID 리스트를 받음
) {
List<BinaryContent> contents = binaryContentService.findAllByIdIn(ids);
return ResponseEntity.ok(contents);
}


@GetMapping("/{id}")
public ResponseEntity<BinaryContent> find(
@PathVariable UUID id) {
BinaryContent content = binaryContentService.find(id);
return ResponseEntity.ok(content);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RESTFul API에서 GET이 이미 자원에 대한 조회의 의미를 내포하기 때문에 /find 굳이 추가할 필요 없습니다!

@RequestMapping(
path = "/create/private",
method = RequestMethod.POST,
consumes = {MediaType.MULTIPART_FORM_DATA_VALUE}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과제에서 요구사항 대로 하신 것 같아 이해하지만, [POST] "/api/channels/public, private" private, public을 Body 값 안에 넣어서 처리하는 방식이 더 깔끔할 것 같긴 합니다. 정말 분리되어야 하면 모르겠지만 거의 구현 코드가 비슷할거거든요

Copy link

@joonfluence joonfluence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +15 to +26
@ExceptionHandler(Exception.class)
public ResponseEntity<ErrorResponse> handleAllExceptions(Exception e) {
ErrorResponse errorResponse = new ErrorResponse(
HttpStatus.INTERNAL_SERVER_ERROR.value(),
e.getMessage(),
LocalDateTime.now().format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)
);

return ResponseEntity
.status(HttpStatus.INTERNAL_SERVER_ERROR)
.body(errorResponse);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

전체 예외를 500대 에러로 표현해도 될까요?
400대, 500대 상태코드들을 한번 공부해보고 적용 고민해보시죠~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

순한맛🐑 마음이 많이 여립니다..

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants