-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
Update instructor eligibility requirements with Get Involved step #2444
Update instructor eligibility requirements with Get Involved step #2444
Conversation
"not possible to add training progress", form.non_field_errors()[0] | ||
) | ||
|
||
def test_form_with_failed_status_requires_notes(self): |
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.
Because this removal confused me when I looked back at the diff:
Despite the name, the content of this test is a duplicate of test_form_invalid_if_trainee_has_no_training_task
, and the intent is covered by test_progress_with_failed_status_requires_notes
elsewhere in this file.
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.
LGTM 👍
I like the attention to modifying the comments too to make them more Carpentries and less specific to a lesson program.
In the interest of time, I'm raising this not against
feature/instructor-checkout-changes
but instead against the branch used in #2431 for review. Will not merge until #2431 is merged.Fixes #2440.
This PR will:
test_eligible_but_not_awarded
test inamy/dashboard/tests/test_instructor_dashboard.py
which was broken by Create "Get Involved" requirement and associated Involvement model #2431test_lesson_contribution_passed
inamy/dashboard/tests/test_instructor_dashboard.py
which will be fixed as part of Add feature for trainees to self-submit "Get Involved" step #2411