SFU/courses command for issue #656 #666
Conversation
|
please address issues outline here |
There was a problem hiding this comment.
This looks wonderful, great work! I love that you gave proof that you did some basic testing of the command.
I've added a few small important changes you brought up (bounds checking & errors), and a bunch of less important changes.
And finally, as an aside, I'd prefer if next time you can keep all your PR docs in github, either in the issue, PR desc, or as PR comments referring to specific lines (wherever makes the most sense! up to you) It's possible to embed images & use html extended markdown for styling in the github markdown, so hopefully you shouldn't be too limited!
|
@EarthenSky I trust I can let you continue to review and then I can come in at the end to merge and ensure the CI and everything works well? |
yes, sounds great! tysm |
|
Okay @modernNeo, I've done a look through it all; everything looks good to me. Feel free to merge when you're able. Also, thanks to @chanchantang for being patient, and for the awesome PR! |
There was a problem hiding this comment.
just add test cases to https://github.com/CSSS/wall_e/blob/master/Test_Cases.md for each success/failure response that this new command can result in and I think this can be merged
Please check this document for PR information