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.

Sign up for GitHub

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

Jump to bottom

refactor: 레시피 저장 리팩터링 #80

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

refactor: 레시피 저장 리팩터링 #80

wants to merge 3 commits into from

Conversation

70825
Copy link
Member

@70825 70825 commented Jun 25, 2024 •
edited
Loading

Issue

✨ 구현한 기능

  • 좋아요O, 좋아요X를 저장O, 저장X로 교체합니다.
  • 레시피북마크요청_생성의 파라미터명을 bookmark로 교체합니다. (현재 favorite으로 설정되어 있음)
  • 네이밍 규칙을 적용합니다. (영어는 bookmark, 한글은 저장으로 통일) → 북마크를 저장으로 변경

📢 논의하고 싶은 내용

    protected void 복수_저장한_레시피_저장(final RecipeBookmark... recipeBookmarksToSave) {
        final var recipeBookmarks = List.of(recipeBookmarksToSave);

        recipeBookmarkRepository.saveAll(recipeBookmarks);
    }

원래 네이밍 규칙이 복수_레시피_저장_저장으로 해야하는데 나중에 테스트 코드에 복수_레시피_저장_저장(...)가 있으면 무슨 내용인지 몰라서 내부 메서드 코드까지 확인할 것으로 보입니다. 그래서 복수_레시피_북마크_저장과 복수_저장한_레시피_저장중에 고민하다가 북마크를 걷어내기로 결정해서 복수_저장한_레시피_저장로 선택했습니다

🎸 기타

⏰ 일정

  • 추정 시간 : 1
  • 걸린 시간 : 0.5

@70825 70825 self-assigned this Jun 25, 2024
@github-actions GitHub Actions
Copy link

Test Results

333 tests  ±0   333 ✅ ±0   18s ⏱️ -1s
204 suites ±0     0 💤 ±0 
204 files   ±0     0 ❌ ±0 

Results for commit cb5482b. ± Comparison against base commit d8fad12.

wugawuga
wugawuga approved these changes Jun 27, 2024
View reviewed changes
Copy link
Contributor

@wugawuga wugawuga left a comment

Choose a reason for hiding this comment

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

일단 approve 는 했습니다.

저장으로 바꾼다는게, 다른 개발자가 우리 프로젝트에 붙어서 한다면 괜찮다고 생각해요.
하지만 지금은 저희가 쭉 개발할건데 북마크는 다 알고있어서 그대로 가도 괜찮지 않나? 라는 생각을 해보았습니다.

북마크 -> 레시피 저장

취향차이인것 같아서 승인했습니다!
저는 뭐든 상관없는것 같아요 고생했어 다빈짱

@70825
Copy link
Member Author

70825 commented Jun 27, 2024

오호 그러면 한 명 더 의견을 들어보고 다시 수정할지말지 결정합시다

사실 저도 이거 수정하면서 저장이라는 말이 save랑 뜻이 겹쳐서 나중에 헷갈릴만 하다는 생각이 많이 들었음요

Go-Jaecheol
Go-Jaecheol approved these changes Jul 11, 2024
View reviewed changes
Copy link
Contributor

@Go-Jaecheol Go-Jaecheol left a comment

Choose a reason for hiding this comment

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

이전 PR에서 얘기했던 것처럼, 실제 UI에서도 저장이라고 하고 있고 프론트에서도 저장이라고 하고 있는 것 같아서 같이 통일하는 게 좋아보이긴 해요.

근데 우가 말대로 지금 백엔드 인원들끼리는 용어 정리가 잘 되어있고, 새로운 사람이 들어와도 잘 전달만 되면 문제는 없을 것 같기도 하네요. (당장 꿀조합이랑 레시피 용어도 ㅎㅎ,,)

그래도 둘 중에 하나를 꼭 골라야 한다면 전 북마크 -> 레시피 저장으로 프론트랑 용어 통일 하는 게 좋아보입니다. 😎

@70825
Copy link
Member Author

70825 commented Jul 19, 2024

저희 그.. 다음 회의 있겠죠..??

지금 내용이 중요한 PR은 아니기 때문에 회의 시간에 이야기 해봅시다

[회의 내용] 북마크 → 레시피 저장으로 이름 변경시의 어려움 발생

  1. 레시피 저장이라는 네이밍 자체가 기존에 레시피 글 등록을 하는 레시피 저장과 의미가 혼용되고 있음
    • 나중에 코드 변경시 레시피 저장이라는 단어가 2가지 의미로 사용되어서 테스트코드 작성시 혼동할 수 있어 보입니다
  2. 해결법?
    • 레시피 저장 그대로 유지
    • 북마크로 다시 바꾸기
    • 더 좋은 방법..??

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

@wugawuga wugawuga wugawuga approved these changes

@Go-Jaecheol Go-Jaecheol Go-Jaecheol approved these changes

@hanueleee hanueleee Awaiting requested review from hanueleee

Assignees

@70825 70825

Labels
🔧 Refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

레시피 저장, 저장한 레시피 조회 리팩터링
3 participants
@70825 @Go-Jaecheol @wugawuga