-
Notifications
You must be signed in to change notification settings - Fork 240
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
Add stage
argument to tell checking functions what type of check is being performed
#610
Conversation
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.
Looks great, thank you! I think the changes in the docs do a good job showcasing the improvement!
R/exercise.R
Outdated
) | ||
message(msg) | ||
rlang::return_from(rlang::caller_env(), exercise_result_error(msg)) | ||
# Don't throw an error if the only missing argument is `stage` |
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.
It'd be better if this comment explained why we're making the exception for stage
.
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.
How does 5b36cf1 look?
Oh and let's bump the dev version for this one |
…r function missing `stage` in its signature
@gadenbuie Dev version bumped! Should this go in the |
Calls to
try_checker()
withinevaluate_exercise()
gain the argumentstage
– one of"code_check"
,"error_check"
, or"check"
– which is passed to the checking function to indicate which kind of check is being performed.This simplifies the implementation of custom checking functions, which no longer need to infer the stage from other arguments. For example, the example function in the
learnr
docs becomes:For backwards compatibility, if the function signature of the checking function does not include
stage
or...
,stage
is not passed.Closes #567.