Course ECP Command #171
Merged
Course ECP Command #171
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Made xkcd_desc an f string, added spoiler `||` on either side so when it's posted to discord it spoilers the text.
Attempting to create a command that takes in a course code, selected year, semester, mode, and campus and then provides the link to the course profile. Currently it defaults to the current semester and I'm yet to work out how to make it not default.
Need to format the embedded and allow it to take in multiple courses
It assume when requesting you want the same semester, year, mode and campus for all courses.
49Indium
requested changes
Oct 24, 2023
There was a problem hiding this comment.
Glad to see your first PR! Overall great style. I like the detail in the description of the command. and the error checking is superb (I like how the error messages are informative based on the type of error). There is some code duplication that would be nice if removed, but it really stems from past poor design choices (of mine). Looks like a great command for the bot to have.
- Fixed typo - Clarified error message - Changes wording of the else statement that in theory should never happen because of the exceptions.
Unsure if this will cause issues, doesn't look like it and improve any future use of checking what the estimated current semester is.
|
Changes as requested made, no clue what to do with Black though |
andrewj-brown
previously approved these changes
Oct 27, 2023
|
@andrewj-brown or @49Indium Black has been sorted, thank ya'll |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Sick of closing Discord for 30 seconds to search for the course code? Great news that is no more! With the new
/courseecpcommand, you can request the ECPs for up to four courses for a semester, year, mode, and campus. It assumes the courses are offered in the same semester and that they all are the same mode of delivery and campus. If semester, year, mode and campus are not provided, it defaults to the current semester and year.Further development of this command could be removing those assumptions and making it more complex for what it takes as inputs.