Skip to content

Conversation

@louietyj
Copy link
Collaborator

No description provided.

@louietyj louietyj requested a review from yangshun April 23, 2018 12:00
@louislai
Copy link

👍

@yangshun
Copy link
Member

I'm happy and sad at the same time that the scripts still need to be maintained. Thanks!

@yangshun yangshun merged commit ecb3cbb into nusmodifications:master Apr 23, 2018
@louietyj
Copy link
Collaborator Author

Wow didn't get flamed for the great big hack

@yangshun
Copy link
Member

Didn't really review because I can't test it and if it works, it works. I wouldn't import a time library just for this anyway. The way you did it was fine in the context of a one-off script.

#movefast

Copy link
Member

@yangshun yangshun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed

day: 'SUNDAY',
start_time: '1400',
end_time: '1600'
start_time: times['start_time'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

times.start_time

start_time: '1400',
end_time: '1600'
start_time: times['start_time'],
end_time: times['end_time']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

times.end_time

if (week == 7) continue;
// Grading claims
for (var ps = 0; ps < 5; ps++) {
var ps_hours = ps_students[ps] * ps_rates[ps];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vars are function scope, so avoid using vars in a for-loop. Use let or wrap the body in an IIFE.

@louietyj
Copy link
Collaborator Author

🌚

start_time: '1200',
end_time: '1400'
});
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/nit these spaces lol

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tsk @louietyj never configure editor to trim spaces on save

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notepad 😒

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants