-
Notifications
You must be signed in to change notification settings - Fork 15
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
List advisory Location #630
base: master
Are you sure you want to change the base?
Conversation
PyGithub | ||
six # for some reason google cloud needs this |
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.
Shouldn't need this when using python3? Can you post the error you are seeing?
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.
This runs fine on my own local server, but errors for some reason in the test system on Github
static/js/index.js
Outdated
@@ -460,13 +460,17 @@ function renderStudent(studentObj) { | |||
var advisoryTag = '<p><iron-icon icon="icons:perm-identity"></iron-icon>' + | |||
"Advisor: " + studentObj.advisor.charAt(1).toUpperCase() + | |||
studentObj.advisor.slice(2) + "</p>"; | |||
var advisoryLocationTag = '<p><iron-icon icon="icons:room"></iron-icon>' + | |||
"Advisory Room: " + studentObj.classes[0][8].room + "</p>"; |
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.
This feels a bit fragile, and might not work well if the student has an unusual course load. Maybe add a function to get a class by name, and we can search for "Advisory"?
@@ -26,6 +26,14 @@ function renderGitHubCommits() { | |||
}); | |||
document.getElementById("GHUpdateText").innerHTML = updated; | |||
} | |||
function getCourseByName(studentClasses, name) { |
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.
The name
parameter isn't being used here...
fixes #629