-
Notifications
You must be signed in to change notification settings - Fork 0
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
Issue/168/update best review #169
base: dev
Are you sure you want to change the base?
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.
Code review by ChatGPT
"bin": { | ||
"uuid": "dist/esm/bin/uuid" | ||
} | ||
}, | ||
"node_modules/uuid-random": { | ||
"version": "1.3.2", | ||
"resolved": "https://registry.npmjs.org/uuid-random/-/uuid-random-1.3.2.tgz", |
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.
코드 패치 리뷰를 진행하겠습니다. 전체적으로 패치된 내용은 @nestjs/schedule
, cron
, luxon
, uuid
등의 패키지를 추가한 것 같습니다. 다음은 몇 가지 고려사항입니다:
-
버전 호환성:
@nestjs/schedule
의 경우peerDependencies
에@nestjs/common
,@nestjs/core
의 버전 호환성이 명시되어 있습니다. 현재 사용 중인 NestJS의 버전이 요구사항을 충족하는지 확인해야 합니다. 만약 충족하지 않는다면, 런타임 시 오류가 발생할 수 있습니다.
-
패키지 의존성 확인:
uuid
와luxon
패키지는 각각 다른 패키지에서 직접적으로 의존성을 가질 수 있습니다. 만약 두 패키지 간에 버전 충돌이 발생하면 예기치 않은 동작을 초래할 수 있습니다. 이 부분도 종속성 트리를 검토하여 확인하는 것이 좋습니다.
-
보안 업데이트:
- 패키지의 최신 버전이 보안 문제를 해결했는지 확인하는 것이 중요합니다.
cron
,luxon
,uuid
같은 패키지를 추가하였으므로 이들의 보안 공지 및 이력을 체크하세요.
- 패키지의 최신 버전이 보안 문제를 해결했는지 확인하는 것이 중요합니다.
-
패키지 LICENSE:
- 각 패키지의 라이센스를 확인하여 프로젝트의 라이센스 정책과 맞는지 검토해야 합니다. 모든 패키지가 "MIT" 라이센스를 갖고 있지만, 향후 패키지 추가 시 주의가 필요합니다.
-
버전 고정:
dependencies
및devDependencies
에서 버전을 고정할 때 주의해야 합니다. 예를 들어,@nestjs/schedule
과 같은 패키지를 성공적으로 적용했으나 나중에 이러한 패키지의 마이너 버전에서 breaking changes가 발생할 수 있습니다. 가능한 한^
표기를 사용하는 것이 좋지만, 모든 경우에 대해 테스트가 필요합니다.
-
테스트 세트업:
- 패치 후에 모든 관련 기능이 정상 작동하는지 확인하기 위한 테스트 케이스를 작성하거나 실행해야 합니다. 특히 스케줄러는 시간에 따른 특정 로직을 처리하므로, 꼼꼼한 테스트가 중요합니다.
이러한 점들을 고려하여 코드 패치의 안정성을 높이는 데 도움이 되길 바랍니다. 추가적인 질문이나 설명이 필요하시면 언제든지 말씀해 주세요!
@@ -142,5 +143,6 @@ | |||
}, | |||
"overrides": { | |||
"glob": "^9" | |||
} | |||
}, | |||
"packageManager": "[email protected]+sha512.1acb565e6193efbebda772702950469150cf12bcc764262e7587e71d19dc98a423dff9536e57ea44c49bdf790ff694e83c27be5faa23d67e0c033b583be4bfcf" | |||
} |
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.
코드 패치에 대한 간단한 검토를 진행하겠습니다.
-
의존성 추가:
@nestjs/schedule
패키지를 추가한 것은 작업 스케줄링을 위한 기능을 제공할 수 있어 유용합니다. 그러나 이 패키지를 사용하는 부분이 코드의 다른 부분에서도 필요할지 검토해보는 것이 좋습니다. 사용하지 않으면 필요 없는 의존성이 될 수 있습니다.
-
버전 관리:
@nestjs/schedule
의 버전이^4.1.2
로 업데이트 되었는데, 해당 버전이 현재 프로젝트와 호환되는지 확인하는 것이 중요합니다.의존성 충돌 위험이 있을 수 있습니다.
-
Overrides 구문:
- "overrides" 항목에서 "glob": "^9"을 사용하고 있습니다. 이 설정이 다른 의존성과의 호환성에 영향을 미치지 않는지 테스트해보는 것이 좋습니다.
-
packageManager 항목 추가**:
packageManager
항목을 추가한 것은 좋지만, 버전과 해시값이 정확한지 확인해야 합니다. 잘못된 값이 들어가면 패키지 관리에 문제가 생길 수 있습니다.
-
코드 가독성:
- 변경 사항이 크지 않으므로, 전반적인 가독성에는 큰 문제가 없어 보입니다. 다만, 장기적인 유지보수를 위해서 관련된 변경 사항에 대한 주석이나 문서화가 필요할 수 있습니다.
-
테스트:
- 새로운 패키지를 포함했기 때문에, 해당 패키지가 정상적으로 작동하는지 확인하기 위한 테스트 케이스가 필요합니다. 추가된 기능이 예상대로 작동하는지 확인해보세요.
결론적으로, 버전 호환성과 의존성 사용에 대한 충분한 검토 및 테스트가 필요합니다. 전반적으로 문제가 있어 보이지 않지만, 추가적인 검토가 필요할 수 있습니다.
export type WithLectures = Prisma.review_reviewGetPayload< | ||
typeof WithLectures | ||
>; | ||
|
||
export namespace EReviewVote { | ||
export const Basic = | ||
Prisma.validator<Prisma.review_reviewvoteDefaultArgs>()({}); |
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.
코드 패치에 대한 간단한 리뷰를 제공하겠습니다.
-
코드 가독성: 새로운
WithLectures
상수와 타입이 추가되었습니다. 전반적으로 코드가 잘 정리되어 있어 가독성이 좋습니다. 하지만 각 프로퍼티의 역할이나 사용 목적에 대한 주석을 추가하면 다른 개발자들이 이해하는 데 도움이 될 것입니다. -
정의 및 사용 장소:
WithLectures
상수와 타입이EReview
네임스페이스에 잘 포함되어 있지만, 전체적인 구조에서 이 상수가 어디에서 사용될지 명확하지 않습니다. 만약 이 상수가 다른 모듈 또는 네임스페이스에서 사용될 가능성이 있다면, 예시나 문서를 추가해주는 것이 좋습니다. -
버그 리스크: 현재 코드에는 명백한 버그가 있는 것 같지는 않습니다. 하지만
ELecture.Basic
이 유효한지 확인해야 합니다. 만약Basic
이 정의되지 않은 상태에서 접근하게 되면 런타임 오류가 발생할 수 있습니다. -
타입 안정성:
WithLectures
의 타입이Prisma.review_reviewGetPayload
를 기반으로 하고 있으므로 타입 안정성은 잘 유지되고 있는 것 같습니다. 그러나 언제든지 Prisma의 API가 변경될 수 있으니, 이러한 의존성에 유의해야 합니다. -
테스트: 이 새로운 추가 기능에 대해 테스트가 필요한지 검토해야 합니다.
WithLectures
와 관련된 단위 테스트가 없다면, 나중에 버그가 발생할 가능성을 높일 수 있습니다. -
추가적인 개선: 자주 사용하는 패턴에 대한 헬퍼 함수나 유틸리티를 도입하면 코드의 재사용성과 유지보수성이 향상될 수 있습니다.
전반적으로 코드는 명확하고 잘 작성되어 있지만, 위에 언급한 몇 가지 사항을 고려하면 코드의 품질과 안정성을 더 높일 수 있을 것입니다.
await this.reviewsService.updateBestReviewsCron(); | ||
return { message: 'BestReviews update triggered successfully' }; | ||
} | ||
|
||
@Public() | ||
@Get(':reviewId') | ||
async getReviewInstance( |
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 문을 정리하고, 불필요한 중복을 제거하여 코드를 깔끔하게 유지했습니다.
-
중복 제거:
ReviewProhibited
모듈의 import 순서가 변경된 것은 일관성을 유지하는 데 도움이 됩니다. 불필요한 중복을 제거했습니다. -
메소드 추가:
triggerUpdateBestReviews
메소드가 추가되었습니다. 이 메소드는 Best Reviews의 수동 업데이트를 트리거하며, 잘 작성된 것으로 보입니다. 하지만, 이 메소드에서console.log
를 남기는 것이 운영 환경에서는 로그가 남을 수 있으므로, 프로덕션 코드에는 로깅을 위한 적절한 방법(Promise와의 실패 처리 및 레벨링)을 사용하는 것이 좋습니다. -
에러 처리:
triggerUpdateBestReviews()
메소드에서 오류 발생 시 적절한 에러 핸들링을 구현하는 것이 좋습니다. 예를 들어,await this.reviewsService.updateBestReviewsCron();
호출 시 실패할 경우 적절한 에러 메시지를 클라이언트에게 반환하도록 수정할 필요가 있습니다. -
추가적인 유효성 검사:
triggerUpdateBestReviews()
메소드에 접근을 제한하기 위해 권한 검사를 추가하는 것을 고려해볼 수 있습니다. 특정 사용자만 이 메소드를 호출할 수 있도록 설정하면 보안에 도움이 될 것입니다. -
주석 추가: 새로운 메소드에 대한 주석을 추가하면 유지 보수에 도움이 될 것이며, 다른 개발자가 이해하는 데 수월할 것입니다. 메소드의 목적이나 사용 방법에 대한 간단한 설명을 덧붙이는 것이 좋습니다.
이와 같은 수정사항을 고려하면 코드의 품질과 안정성을 높일 수 있을 것입니다.
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.
@Gerbera3090 이렇다고 하네요. console.log는 지워주세용
); | ||
|
||
console.log('BestReview updated by scheduled job'); | ||
} | ||
} |
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.
코드 리뷰를 진행하겠습니다. 전체적으로 동작 로직은 명확하게 작성되어 있습니다. 그러나 다음과 같은 개선 사항 및 버그 위험 요소들이 있습니다:
-
명확한 타입 정의:
calculateKey
,getBestReviews
함수에서 매개변수와 반환 값에 대해 타입을 명시하는 것이 좋습니다. 특히getBestReviews
에서likedCount
,mostLikedReviews
등의 값이 어떻게 사용되는지 명확히 하는 것이 가독성 향상에 기여합니다.
-
상수화:
0.85
와 같은 상수 값은 의미를 명시할 수 있는 이름을 가진 상수로 추출하여 사용하는 것이 좋습니다. 이렇게 하면 코드의 가독성이 향상되고, 이후에 수정할 일이 생겼을 때 유지보수가 용이해집니다.
-
에러 처리:
await
를 사용하는 부분에서 에러가 발생할 경우, 적절한 예외 처리가 이루어지지 않고 있습니다. 예를 들어, 데이터베이스 소스에서 리뷰를 가져오지 못하는 상황을 고려하여 적절한 예외 처리를 추가하는 것이 좋습니다.
-
정렬 및 필터링 로직:
mostLikedReviews
를 생성하기 위한sort()
와slice()
는 비효율적일 수 있습니다. 특히reviews
배열이 클 경우 성능 이슈가 발생할 수 있으므로, 필터링 및 정렬 방식을 검토해 보는 것이 좋습니다.
-
로그 개선:
- 로그 메시지가 기본적인 상태를 출력하고 있으나, 현재 처리 중인 리뷰의 수나 작업 진행 상태 등을 추가하여 더욱 유용한 로그 정보를 제공할 수 있습니다.
-
Cron 표현식의 설명:
- Cron 스케줄링 작업의 표현식에 대해 주석으로 설명을 추가하였으나, 다른 개발자가 이 코드를 읽을 때 어떤 주기로 실행되는지를 명확히 이해할 수 있도록 보다 구체적인 설명을 추가하는 것이 좋습니다.
-
성능 최적화:
getBestReviews
에서 두 개의 배열 (mostLikedReviews
,latestReviews
)를 결합할 때, 이 배열들의 길이에 따라 불필요한 스플라이스가 발생할 수 있습니다. 결과적으로 항상 최대 길이로 생성되고, 필요에 따라 자르기보다는 조건에 따라 생성하는 방식으로 수정할 수 있습니다.
이러한 지점들을 고려하면 코드의 품질과 유지보수성이 높아질 것으로 생각됩니다. 코드를 잘 작성하셨습니다. 앞으로도 계속 개선하는 방향으로 나아가시길 바랍니다!
ALTER TABLE `review_humanitybestreview` ADD CONSTRAINT `review_humanitybestreview_review_id_fkey` FOREIGN KEY (`review_id`) REFERENCES `review_review`(`id`) ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
||
-- AddForeignKey | ||
ALTER TABLE `review_majorbestreview` ADD CONSTRAINT `review_majorbestreview_review_id_fkey` FOREIGN KEY (`review_id`) REFERENCES `review_review`(`id`) ON DELETE CASCADE ON UPDATE CASCADE; |
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.
이 코드 패치는 두 개의 인덱스를 생성하고, 두 개의 외래 키 제약 조건을 추가하는 SQL 스크립트입니다. 다음은 코드 리뷰 및 개선 사항입니다:
-
인덱스 생성:
- 인덱스 이름이 명확하고, 인덱스가 적용될 컬럼도 명확하게 지정되어 있으므로 좋습니다.
- 인덱스가 필요한 이유나 목적이 주석으로 추가되면 더 이해하기 쉬울 것입니다. 예를 들어, 성능 최적화나 쿼리 개선을 위한 것인지 명시하면 좋습니다.
-
외래 키 제약 조건:
- 외래 키의 참조 테이블인
review_review
의 컬럼(id
)이 적절히 설정되어야 합니다. 만약 해당 컬럼이 기본 키가 아니라면 오류가 발생할 수 있습니다. ON DELETE CASCADE
와ON UPDATE CASCADE
설정이 비즈니스 로직에 맞는지 확인해야 합니다. 이러한 설정은 관련된 데이터가 삭제/수정될 때 전파되어 영향을 미칠 수 있으므로 신중한 검토가 필요합니다.
- 외래 키의 참조 테이블인
-
에러 및 예외 처리:
- 독립적으 로 실행할 수 있도록 제약 조건 추가와 인덱스 생성이 순차적으로 성공할 것이라는 가정 하에 작성되어 있습니다. 실제로는 각 쿼리 실행 후 에러 처리 로직이 필요할 수 있습니다.
- 이러한 제약 조건 추가가 실패할 경우, 전체 작업이 롤백되도록 트랜잭션을 사용하는 것을 고려해 보세요.
-
코드 유지 관리:
- 인덱스와 외래 키 제약 조건의 이름이 매우 길어져 있습니다. 이는 코드 유지보수 시 불편할 수 있습니다. 더 간결한 이름을 고려해보는 것이 좋습니다.
-
주석 사용:
- 각 SQL 명령어 앞에 주석이 기본적으로 제공되지만, 구체적인 설명이 포함되면 좋습니다. 예를 들어, "이 인덱스는 리뷰 검색 성능을 높이기 위해 생성되었습니다."와 같은 설명을 추가하면 코드의 의도가 분명해질 것입니다.
결론적으로 코드 구조나 명명 규칙은 적절하나, 비즈니스 로직과 성능 고려를 위해 추가적인 검토가 필요합니다. 에러 처리 및 유지 보수성을 높이기 위한 개선이 필요합니다.
return this.prisma.review_majorbestreview.createMany({ | ||
data: reviews.map((review) => ({ review_id: review.reviewId })), | ||
}); | ||
} | ||
} |
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.
코드 패치에 대한 간단한 리뷰를 제공하겠습니다.
코드 리뷰
-
명확한 함수 이름: 각 함수의 이름은 기능을 잘 설명합니다.
getHumanityReviews
와getMajorReviews
는 각각 인문학 및 전공 리뷰를 가져오는 기능에 대해 명확합니다. -
검증 추가:
addHumanityBestReviews
와addMajorBestReviews
메서드는 리뷰 배열을 받아 데이터베이스에 추가합니다. 이 배열이 비어있거나 유효하지 않은 경우 예외 처리가 필요할 수 있습니다. 빈 배열을 추가하려고 할 때 데이터베이스 호출은 비효율적일 수 있습니다. -
에러 핸들링: 현재 코드에는 에러 핸들링 로직이 없습니다. 데이터베이스 작업에서 예외가 발생할 수 있으므로, try-catch 문을 통해 에러를 처리하고 사용자에게 적절한 피드백을 제공해야 합니다.
-
주석 추가: 코드 각 섹션에 대한 주석이 잘 작성되어 있으며, 기능에 대한 개요를 제공합니다. 추가적으로 각 함수의 반환 값이나 예외가 발생할 수 있는 경우에 대해 더 상세한 주석을 추가하는 것도 좋습니다.
-
성능 고려:
getMajorReviews
에서 HSS 과목이 아닌 모든 리뷰를 검색하는 방식이 성능에 영향 미칠 수 있습니다. 대량의 데이터에 대해 쿼리가 느려질 수 있으므로, 인덱스를 추가하거나 쿼리를 최적화하는 것을 고려해야 할 수 있습니다. -
타입 정의 강화:
addHumanityBestReviews
및addMajorBestReviews
에서reviews
매개변수의 타입을 좀 더 구체화할 수 있습니다. 현재reviewId
가 숫자 형식임을 명시했지만, 이 외에 필요한 속성이 있는 경우 포함할 수 있습니다.
버그 위험
clearHumanityBestReviews
와clearMajorBestReviews
함수는 데이터 삭제 작업을 수행하므로, 이러한 함수가 호출될 때 의도치 않은 데이터 삭제를 방지하기 위해 추가적인 확인을 요구할 수 있습니다.createMany
메서드에 전달된 데이터에 대해 올바른 형식인지 검증하지 않으면 데이터베이스 오류를 일으킬 수 있습니다.
이와 같은 피드백을 바탕으로 코드를 보강하고 안정성을 높일 수 있을 것입니다.
@@ -242,6 +248,9 @@ model review_review { | |||
lecture subject_lecture @relation(fields: [lecture_id], references: [id]) | |||
writer session_userprofile? @relation(fields: [writer_id], references: [id]) | |||
review_reviewvote review_reviewvote[] | |||
review_majorbestreview review_majorbestreview? @relation | |||
review_humanitybestreview review_humanitybestreview? @relation | |||
|
|||
|
|||
@@unique([writer_id, lecture_id], map: "review_comment_writer_id_af700a5d_uniq") | |||
@@index([written_datetime], map: "review_comment_e5e30a4a") |
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.
코드 패치에 대한 리뷰를 진행하겠습니다.
-
관계 설정:
review_humanitybestreview
및review_majorbestreview
모델에서review
필드가 제대로 생성되었습니다. 그러나, 이 관계에 대한 충돌 가능성을 고려해야 합니다. 두 모델이 동일한review_id
를 공유할 경우, 외래 키 제약 조건이 위반될 수 있습니다. 각각의 모델에 대해 어떤 상황에서onDelete: Cascade
가 필요한지 명확히 정리하는 것이 좋습니다.
-
인덱스 추가:
- 인덱스를 추가한 것은 성능 측면에서 긍정적입니다. 그러나
review_id
가 이미 기본 키로 설정되어 있는 상태에서 중복 인덱스를 생성한 것은 불필요할 수 있습니다. 기본 키 자체가 인덱스를 제공하므로, 추가적인 인덱스는 성능 향상에 크게 기여하지 않을 수 있습니다.
- 인덱스를 추가한 것은 성능 측면에서 긍정적입니다. 그러나
-
모델 명명 규칙:
- 모델명을 통해 어떤 기능이나 목적을 갖는지 알 수 있도록 더 직관적인 명명 규칙을 사용하는 것이 좋습니다. 예를 들어,
review_majorbestreview
와review_humanitybestreview
를 더 명확한 이름으로 변경하면 코드 가독성이 향상될 것입니다.
- 모델명을 통해 어떤 기능이나 목적을 갖는지 알 수 있도록 더 직관적인 명명 규칙을 사용하는 것이 좋습니다. 예를 들어,
-
주석 및 문서화:
- 각 모델과 필드에 대한 적절한 주석을 추가하는 것이 좋습니다. 관계 설정이나 각 필드의 용도에 대한 설명이 추가되면 코드 이해도가 높아질 것입니다.
-
테스트 케이스:
- 코드 변경 후 영향을 받는 부분에 대한 테스트 케이스를 작성하여, 변경 사항이 기존 기능에 악영향을 미치지 않는지 확인해야 합니다.
-
성능 고려사항:
- 많은 수의 관계를 설정하면 성능에 영향을 줄 수 있으므로, 이러한 관계가 필요한지 다시 검토해 볼 필요가 있습니다.
종합적으로 보았을 때, 전반적으로 잘 작성된 코드이나, 몇 가지 개선 사항과 주의해야 할 점이 있습니다. 이러한 사항들을 고려하여 더 안정적이고 효율적인 코드를 작성하길 바랍니다.
@@ -174,4 +175,68 @@ export class ReviewsService { | |||
): Promise<EReviewVote.Basic> { | |||
return await this.reviewsRepository.upsertReviewVote(reviewId, user.id); | |||
} | |||
|
|||
@Cron('0 0 * * *') // 매일 자정에 실행하는 Cron 작업 설정: 과거 update-best-reviews.py를 계승 |
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.
이거 이렇게 해놓으면 cron 도는거에요?
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.
이거 서버에서 지금 실행되는거죠..? 저희가 서버가 여러개라서 이렇게 해놓으면 모든 서버에서 다 돌아갈거에요.
그래서 지금 secure-VM에서 크론을 돌고 있는거기도 하고요, 거기는 요청을 한 군데로만 보내니까요.
지금 상황은 그런데 저거 쓰는거는 긍정적인 방향인거 같긴 합니다.
저기 sync 모듈을 서버 안에서 처리하지 말고 아예
- 모노 레포 형식으로 만들기
- 필요한 prisma 타입 및 interface 공유하기
- cron 붙여서 secureVM에서 바로 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.
아마 그러면 secure-vm에 cron에 열어뒀던 get요청 보내는 코드를 추가하면 될 것 같습니다. 지금 cron은 주석 해두고요.
function calculateKey(review: EReview.WithLectures): number { | ||
const baseYear = new Date().getFullYear(); | ||
const lectureYear = review.lecture.year; | ||
const yearDiff = baseYear - lectureYear > 0 ? baseYear - lectureYear : 0; | ||
return Math.floor( | ||
(review.like / (review.lecture.audience + 1)) * | ||
Math.pow(0.85, yearDiff), | ||
); | ||
} | ||
|
||
function getBestReviews( | ||
reviews: EReview.WithLectures[], | ||
minLikedCount: number, | ||
maxResultCount: number, | ||
): review_review[] { | ||
const likedCount = Math.max( | ||
minLikedCount, | ||
Math.floor(reviews.length / 10), | ||
); |
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.
저 calculateKey 함수는 어떤건가요...? 약간 시간이 흐름에 따라 감쇠하는 factor가 있는건가..?
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.
이거 혹시 하고 싶다면...? 그냥 좋아요 개수 / 청중 수를 감쇠로하지 말고, 시간 흐름별 좋아요 개수를 moving average로 해보는건 어때요?
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.
아마 시간이 지날수록 해당 과목이 현재랑은 좀 달라져서 효과는 떨어지니까 그걸 노리고 만들었던 것 같아요
NOT: { | ||
course: { | ||
subject_department: { | ||
code: 'HSS', |
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.
이거 기필도 다 포함되어서 subject_department 긁어와서 in으로 넣는게 낫지 않을까 싶긴합니다.
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.
그리고 EReview.withLectures 만드셨으니까 include에 저렇게 lecture:true를 쓰시는게 아니라 include: EReview.withLectures.include 이렇게 하시면 됩니다
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.
subject_department 테이블 보니까 전공만 따로 뽑는게 없는 거 같더라구요... 또 그냥 학과들만 넣자니 부전공 복수전공도 띄우는 것 같아서 일단은 visible 정도만 추가하겠습니다
어라 린트가 왜 안풀리지 |
Best Review를 업데이트 하는 cron service 및 get 요청으로 켜는 api도 뚫어 뒀습니다.
기존 otlplus 에서 update-best-reviews.py를 cron 작업으로 변환한 것입니다.
테스트 완료하였고, 새롭게 review_majorbestreview와 review_humanitybestreview 테이블의 재설정 확인하였습니다.
주로 리뷰받고 싶은 내용은 다음과 같습니다.
이전의 매우 간단한 태스크 다음으로 하는 태스크라 모르는 점이 많고, 제가 I,E 시스템을 잘 이해하였는지도 의문이 들어, 꼼꼼한 리뷰 부탁드리겠습니다!!!