-
Notifications
You must be signed in to change notification settings - Fork 1
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
refactor: 신입 모집 마감 날짜 수정 #174
Conversation
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.
👍 하나만 확인 부탁드립니다 ~!
export const END_DATE = { | ||
year: 2024, | ||
month: 9, | ||
date: 15, | ||
hours: 15, | ||
minutes: 0, | ||
seconds: 0, | ||
}; |
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.
위의 JS DOC도 최신화 해주면 좋겠네요!
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.
DOC를 최신화 해주실 때 혹시 UTC라서 9시간 차이나는 것도 추가해주실 수 있으신가요??
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.
좋네요! 수고하셨습니다
@@ -17,6 +17,7 @@ import { | |||
applicationDataAtom, | |||
applicationIndexAtom, | |||
} from "../stores/application"; | |||
import { END_DATE } from "../constants/application/28"; |
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.
동적 import 를 사용해도 좋을 것 같습니다! 다만 지금도 충분히 좋아 보입니다!
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.
이제 진짜 본격적이라는 생각이 들어서 벌써 무섭네요 ㄷㄷㄷㄷ....
export const END_DATE = { | ||
year: 2024, | ||
month: 9, | ||
date: 15, | ||
hours: 15, | ||
minutes: 0, | ||
seconds: 0, | ||
}; |
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.
DOC를 최신화 해주실 때 혹시 UTC라서 9시간 차이나는 것도 추가해주실 수 있으신가요??
if ( | ||
Date.now() > | ||
Date.UTC( | ||
END_DATE.year, | ||
END_DATE.month - 1, | ||
END_DATE.date, | ||
END_DATE.hours, | ||
END_DATE.minutes, | ||
END_DATE.seconds | ||
) | ||
) { |
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.
항상... 무섭네요...ㅠㅠㅠ
흠... 이렇게 작성하려면 UTC말도 더 좋은 방법이 있는지 고민해봐도 좋을 것 같다는 생각이 듭니다...
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.
- JS DOC 최신화
- 동적 import로 변경 ( await import로 변경하려 하였으나 useSetAtom과 같은 리액트 훅을 async 안에 작성하면 안된다는 eslint 에러로 인해 require 문으로 변경하였습니다.)
- UTC 고려하여 시간 비교를 수정하였습니다.
- 1차 모집 기간 날짜 수정
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.
👍 고생하셨습니다
주요 변경사항
리뷰어에게...
관련 이슈
closes #173
체크리스트
reviewers
설정label
설정milestone
설정