Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2차 멘토링 코드리뷰용 PR #26

Closed
wants to merge 106 commits into from
Closed

2차 멘토링 코드리뷰용 PR #26

wants to merge 106 commits into from

Conversation

daeun084
Copy link
Member

No description provided.

daeun084 and others added 30 commits October 9, 2024 16:38
…p logic

Log-in and membership registration logic using JWT was implemented.

Related: #1
…ging-in-directory-structure-and-implementing-subscription-logic

[Feature] Setting up and logging in directory structure and implementing subscription logic
…settings

[Feature] Implements Deploy Settings
[Feature/#4] 일기 CRUD 기능 구현
[Chore/#9] 일기 작성 기능 수정
daeun084 and others added 27 commits November 13, 2024 13:51
[Feature/#8] 행동 요령 제안 기능 구현
[Feature/#16] 감정 분석 레포트 기능 구현
[Feature/#20] 감정 분석 레포트 상세 조회 기능 구현
[Feature/#18] 홈 화면 조회 기능 구현
implement OpenAI API connection in service layer.

Related: #14
Generate personalized compliment card content and title based on user diary entries.

Related: #24
…ment-card-feature

[FEATURE] Implement Compliment Card Feature
Now, return Todo Guide at postTodo

Related:
update Todo Guide

Related:
Copy link

@anselmo228 anselmo228 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! Open AI도 잘 활용하고 폴더구조도 잘 짜신것 같습니다.
지금도 충분히 좋지만, 인프라 부분과 서비스 코드 관련하여 남겼으니, 반영하실 부분 반영 하면 좋을 것 같습니다!

run: |
echo "${{ secrets.DOCKER_PASSWORD }}" | docker login -u "${{ secrets.DOCKER_USERNAME }}" --password-stdin

- name: JIB로 Docker 이미지 빌드 및 푸시

Choose a reason for hiding this comment

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

좋습니다 ㅎㅎ

Choose a reason for hiding this comment

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

EC2 스펙을 어떤걸 사용하는지 모르곘으나, t2.micro와 같은 낮은 사양에서는 'docker prune'과 같은 명령어를 통해 디스크를 확보해두는게 안정적입니다!


@RestController
@RequiredArgsConstructor
@RequestMapping("/api/v1/diary")

Choose a reason for hiding this comment

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

RESTFUL 하게 API를 짜는것에 있어, url은 복수형으로 변경하는건 어떨까요?


@Getter
@NoArgsConstructor
public class DiaryAnalysisRequest {

Choose a reason for hiding this comment

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

Param이 한개면 굳이 DTO로 묶은 이유가 있으실까요?? 딱히 문제가 되는건 아니지만 나중 유지 보수를 위해서는 Request Param 단일 변수로 받고 "일기 내용을 입력해주세요." 이부분은 Controller에서 Swagger @parameter로 넣는것도 좋아보입니다!
지금도 좋긴합니다!

Comment on lines +9 to +11
HAPPY("행복", "🥰", 1), DELIGHT("기쁨", "🥳", 2), EXCITING("신남", "😝", 3),
SOSO("보통", "😐", 4),
ANNOYING("짜증", "😫", 5), SAD("슬픔", "😢", 6), ANGRY("화남", "😡", 7);

Choose a reason for hiding this comment

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

!!

Comment on lines +45 to +53
validContentLength(request.getContent());

Long userId = getUserIdFromAuthentication();

User user = findUserById(userId);

String guide = getTodoGuide(request.getContent());

planRepository.save(PlanConverter.toPlan(request.getContent(), user, guide));

Choose a reason for hiding this comment

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

이부분 두줄로 줄일수 있을 것 같습니다!

@daeun084
Copy link
Member Author

고생하셨습니다! Open AI도 잘 활용하고 폴더구조도 잘 짜신것 같습니다. 지금도 충분히 좋지만, 인프라 부분과 서비스 코드 관련하여 남겼으니, 반영하실 부분 반영 하면 좋을 것 같습니다!

확인했습니다 자세한 코드 리뷰 감사합니다 !! 😊

@daeun084 daeun084 closed this Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants