-
Notifications
You must be signed in to change notification settings - Fork 6
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
추가/즐겨찾기/이삭줍기 카운트 추가 #67
base: develop
Are you sure you want to change the base?
Conversation
@junglesub님 브랜치로 이동해서 테스트 하다가 LectureCard의 isBookmarked, isSpike의 상태가 클릭에 반응을 안할때가 있어서 이를 해소하려 보니 기존 코드에 문제가 많아보여서 일단 이 부분 개발은 재공사가 좀 필요해보이네요. 작업하시느라고 고생 많이하셨을 것으로 에상되는데 수고 많으셨습니다!😭🙏 |
@zoomkoding 분명히 작업하면서 작동 테스트를 하고 MR 을 작성했는데 작동이 정상적으로 안되서 저도 많이 당황스럽네요.. 분명히 됐던 것 같아 조금 더 작업을 해보겠습니다! 수정: 기억으로는 검색 화면 그리고 즐겨 찾기 화면 모두 확인 및 테스트를 한걸로 기억되지만 저도 다시 확인해보니 오류가 있는것을 확인했습니다. 관련 수정 진행 후 다시 댓글 남기겠습니다. |
@junglesub님 정말 고생 많으셨습니다! 바로 내보낼 수 있으면 좋겠는데 과목을 삭제하고 추가하는 과정에서 서버쪽에 로드가 엄청나게 걸리는 이슈가 있는데 원인을 파악해서 해결한 후에 서비스에 나가야할 것 같습니다. 아무래도 근본적인 코드에 이슈가 있는 부분인지 좀더 확인이 필요해보이고 이것에 우선적으로 프론트 상태관리 리팩토링을 진행한 후에 진행하면 좋을 것 같다는 생각이 듭니다! 일단 이 PR은 닫고 프론트 상태관리쪽 리팩토링이 끝난 후에 돌아오는 것은 어떨까요? |
저번에 오류 이후로 아직 문제가 있어서 작업중이었는데 Draft 으로 바꿔두는 것을 깜빡 잊어버렸네요.. 하지만 서버는 완성했다고 생각했는데 말씀해주신 로드에 대해서는 저는 문제가 없었던 걸로 기억해서 다시 한번 확인해 봐야할 것 같습니다. 리팩토링 후 작업하는 것 관련해서는 - 로드맵에 상태관리 리펙토링 하시는 것이 계획에 있으신건가요? 회의 시간에 다시 다뤄봐야하는 내용 같은데 저번에는 리팩토링 없이 기능추가 위주로 가자는 이야기가 나와 다시 회의를 진행해봐야할 것 같습니다. 꼭 필요한 핵심 기능이 아니면 연기하는 것도 방법일 것 같습니다. |
네 리팩토링을 할 필요성을 제가 깊이 찾지 못했었는데 막상 코드 내부를 들여다보니 여지가 매우매우매우 많아보이네요. |
resolves #48
기존의 Hook / Class 구조를 최대한 반영하며 기존에 있던 state를 업데이트 하는 방법을 사용했습니다.
업데이트 하던 도중 기능과 관련된 기능을 테스트를 하였고 문제가 없었지만, 기존의 데이터를 수정하는 과정에서 원치 않는 효과 (Side Effect)가 발생 할 수 있으니 이 점을 집중해서 봐주세요!
수정내역
사진