-
Notifications
You must be signed in to change notification settings - Fork 11
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
adding AI minor #946
base: main
Are you sure you want to change the base?
adding AI minor #946
Conversation
|
[diff-counting] Significant lines: 147. |
Visit the preview URL for this PR (updated for commit 08ab9c4): https://cornelldti-courseplan-dev--pr946-nmp79-aiminor-5ifmzzj0.web.app (expires Thu, 28 Nov 2024 20:01:51 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 6d4e0437c4559ed895272bbd63991394f1e0e933 |
src/data/minors/ai.ts
Outdated
{ | ||
name: 'Human-AI Interaction', | ||
description: 'Add Core Course 3', | ||
//TODO: add INFO 3450 exception for students grad in dec 2024 or may 2025 |
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.
Add a space between // and TODO.
We should do a requirement migrations
to accommodate different major requirements based on entry year. You can check out this documentation for it! This would be under a modification
Something along the lines of
then follow the instructors for index.ts
and running the commands in your terminal, then you should be all set!
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.
perhaps same idea for CS 4780/CS3780
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.
thanks for the requirement migration details, hannah! yes - we would want to add something like this. make sure that the file for the AI minor has the current requirements and the migration is for previous requirements
slotNames: ['Course'], | ||
}, | ||
{ | ||
name: 'AI Electives', |
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.
@nidhi-mylavarapu make sure this cross listing (bug?) does not break the functionality.
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.
Right, this was a weird bug -- grabbing a course from the first bucket shouldn't cause the courses in the second bucket to change to their cross listed names. I would look for another major or minor that requires you to select 2/n courses to fulfill a requirement and repeat that functionality.
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.
Hi nice code so far! I'm super excited to finally and my requirements for AI. I think we should also remove some fields such as Course
, ifCodeMatch
, etc, since it was defined but never used. If you don't see it on ur end on vscode, maybe it's because prettier isn't running?
Also added some potential solutions for some of your TODOs
There is a potential? bug for selecting the AI Electives. Will let Nidhi also look at it too. Good job!!
|
||
export default aiMinorRequirements; | ||
|
||
// export const aiMinorAdvisors: AdvisorGroup = { |
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.
instead, should we add [email protected]](mailto:[email protected]). it's not an actual person but it's good contact information.
Summary