-
Notifications
You must be signed in to change notification settings - Fork 1
[25.02.27 / TASK-130] Feature - 빠른 조회 기능 구현 및 오류 수정 #17
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
Conversation
process.env의 동작 방식으로 인해 구조분해할당이 안 된다고 합니다..
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
post = post.sort( | ||
(a, b) => new Date(a.date).getTime() - new Date(b.date).getTime(), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋㅋ이건 지금은 추가하되 추후 뺍시다, 서버 사이드에서 꼭 해줘야하는 건데 ㅠㅠ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동의합니다..
종종 그래프가 튀던 이유가 날짜 정렬 문제여서 급히 핫픽스 느낌으로 넣긴 했는데, 성능상 문제가 생기는 경우도 있을 것으로 보이네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋았던 점
- 이번에는 마이너한 작업들이라 크게 이견없습니다! 점점 고도화 되는 것 같아서 보는게 뿌듯하네요
- 서버사이드에서 처리해야할 것이 FE 에 전가된 것 같아 슬프네유, 지금은 hot-fix 로 넣되, 나중은 빼시죠! (client-side sorting 연산이,, 데이터가 많아지면 부담이라 ㅠ)
requiredEnv
에서process.env
로 바꾸는 계기가 궁금하네요!
원래 process.env가 반복되는 게 아쉬워서 requiredEnv로 통합했는데, 실행해보니 env가 빈 객체로 넘어왔습니다. 자세한 설명은 이 항목의 마지막 부분을 읽어보시면 됩니다! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋았던 점
- 코드의 전체 가독성이 높아져 보기 편했던 것 같습니다!
ps. 고생하셨습니다! 앞으로도 파이팅입니다🔥
}, | ||
}, | ||
x: { axis: 'x', grid: { color: COLORS.BORDER.SUB } }, | ||
y: { axis: 'y', grid: { color: COLORS.BORDER.SUB } }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
훨씬 가독성이 좋은 것 같아요!👍
type: 'View', | ||
}); | ||
const [type, setType] = useState({ start: '', end: '', type: 'View' }); | ||
const [mode, setMode] = useState<ModeType>('none'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋았던 점
- 전체적으로 코드 가독성이 좋아진 것 같습니다.
- 데이터 정렬은 서버 쪽에서 해주는 방향이 좋을거 같은데,, 빨리 추가해보도록 할게요. 말씀하셨던거처럼 성능 이슈가 있을 수도 있을거 같아요. 빠르게 대응해주셔서 감사합니당 :)
진행한 일