-
Notifications
You must be signed in to change notification settings - Fork 5
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
time_json에 실제 시간 추가 #231
time_json에 실제 시간 추가 #231
Conversation
@Hank-Choi 이거 test 가 깨져요 ㅠㅠ |
@Hank-Choi 여전히 깨지고 있습니당 |
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.
전체적으로 좋으나 코멘트를 좀 남겼습니다.
}) | ||
it("timeAndPlaceToJson__success__mergeContinuousCourse", async function () { | ||
assert.deepEqual([{day: 3, start: 9, len: 4, place: '220-317', start_time: '17:00', end_time: '20:50'}], | ||
TimePlaceUtil.timeAndPlaceToJson('목(9-2)/목(11-2)', '220-317/220-317', '목(17:00~18:50)/목(19:00~20:50)')); |
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.
네 힘들었어요 ㅠㅠ
@@ -49,7 +49,8 @@ function getRefLectureFromSugangSnuLecture(year: number, semester: number, lectu | |||
let quota = parseInt(sugangSnuLecture.quota.split(" ")[0]); | |||
|
|||
let parsedClassTime = parseClassTime(sugangSnuLecture.class_time); | |||
let timeJson = TimePlaceUtil.timeAndPlaceToJson(parsedClassTime, sugangSnuLecture.location); | |||
const realClassTime = sugangSnuLecture.class_time | |||
let timeJson = TimePlaceUtil.timeAndPlaceToJson(parsedClassTime, sugangSnuLecture.location, realClassTime); |
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.
뒤에서 변경 없으면 const 쓰면 어떨까요? + 강박적으로 지킬 필욘 없지만 보여서 언급하자면 위 라인에서 마지막 ; 찍어줘도 좋을 듯 합니다.
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.
이거 그냥 기존에 있던 코드는 냅뒀어요ㅋㅋ
다 const로 바꿔버릴까 하다가 그냥 한도 끝도 없을 것 같아서..
@@ -39,7 +39,7 @@ describe("SugangSnuLectureServiceIntegrationTest", function() { | |||
course_title: '그린리더십 인턴십', | |||
credit: 3, | |||
class_time: '금(6-3)', | |||
class_time_json: [ { day: 4, start: 6, len: 3, place: '220-202' } ], | |||
class_time_json: [ { day: 4, start: 6, len: 3, place: '220-202', start_time: '14:00', end_time: '16:50' } ], |
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.
이 PR 에 어울리는 얘기는 아니지만, KST 기준으로 DB 에 저장하는 거 당연히 괜찮은 선택이겠죠?
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.
넹 스트링으로 저장해서 일단은
@@ -31,15 +32,20 @@ export function timeAndPlaceToJson(timesString: string, locationsString: string) | |||
} | |||
} | |||
|
|||
let classes = times.map((time, idx) => { | |||
let classes = times.map((time, idx) => { |
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.
이 친구도 혹시 const 안 되나여?
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.
마찬가쥐
@@ -63,11 +69,13 @@ export function timeAndPlaceToJson(timesString: string, locationsString: string) | |||
|
|||
// Merge same time with different location | |||
// Merge continuous time with same location | |||
// TODO: 연속된 강의 머지기능 유지 논의 |
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.
한 강의가 같은 강의실에서 연속된 시간에 존재하면 머지하는 로직이 있어요
예를 들어 자구? 301 101호에서 13시부터 14:50까지 강의 15시부터 15:50까지 실습이면 그냥 13시~15:50 자구 이런식으로 뜨거든요
그거 실제시간으로 바꿀 때 나눌 필요가 있을까? 하는 얘기입니다
start_time: null, | ||
end_time: null |
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.
여기 null 인 것들은 실제 있을 수 있는 케이스인 건가요?
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.
아뇨 불가능합니다 그냥 이 테스트에서 중요한 부분 아니라서 패쓰해쓰요..
그렇습니다
#227 에 기존 강의 마이그레이션 스크립트도 두었습니다. < 여기 포함된 배치잡이 잘 돌면 앞으로 쓸모 없는 스크립트