implement duplicate schedule checking in schedule generator #974
Conversation
|
[diff-counting] Significant lines: 51. |
|
Visit the preview URL for this PR (updated for commit dbca27e): https://cornelldti-courseplan-dev--pr974-remove-schedule-gen-ssinnx1g.web.app (expires Thu, 06 Mar 2025 19:34:08 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 6d4e0437c4559ed895272bbd63991394f1e0e933 |
…tor" This reverts commit 12c2cd3.
ejcheng
changed the title
| @@ -118,6 +130,7 @@ import Course, { | |||
| Timeslot, | |||
| } from '@/schedule-generator/course-unit'; | |||
| import Requirement from '@/schedule-generator/requirement'; | |||
| import _ from 'lodash'; | |||
There was a problem hiding this comment.
should move this line between line 120 and 121.
| return false; | ||
| } | ||
| const firstScheduleEntries = Array.from(first.schedule.entries()); | ||
| const secondScheduleEntries = Array.from(first.schedule.entries()); |
There was a problem hiding this comment.
I believe there's a slight bug. It should be const secondScheduleEntries = Array.from(first.schedule.entries()); , or else you're just comparing the same entries from first, then this function always returns True. Let me know if that's not the case though!
There was a problem hiding this comment.
Can also add a length check to return false early aswell.
There was a problem hiding this comment.
Hi Eric. I left a few comments for this PR. I believe there is a slight bug for isEqual. Also, I think we should discuss how many pages we want the scheduler generator to output, because if we have more than 10+ pages, that might be too much for the user. But I also agree that we shouldn't always produce 5 pages. Good job!
Summary
This is a WIP PR implementing duplicate checking for schedules in the CoursePlan schedule generator.
Problems, todos, and future considerations: