Refactor site course enrollment filtering. - #16179
Conversation
|
jenkins run python |
7a94e14 to
f713f20
Compare
f713f20 to
112938e
Compare
| # enrollments, because it could have been a data push snafu. | ||
| course_enrollments = list(get_course_enrollments(user, course_org_filter, org_filter_out_set)) | ||
| # get the org whitelist or the org blacklist for the current site | ||
| site_org_whitelist, site_org_blacklist = get_org_black_and_whitelist_for_site(user) |
There was a problem hiding this comment.
@douglashall: I decided to try to fix this code that I had written a while back to not lose it.
I don't know if this rename is a good idea, because the names course_org_filter and org_filter_out_set are used elsewhere.
The options are:
- Leave the code as it was.
- Go with this new code with renames. (I'm game.)
- Do a version of this PR with refactor and updated comments, but without renaming these variables. (I'm sort of game, but I need to move on.)
- Change to the new names everywhere. (I'm not game.) :)
Let me know what you think. Thanks.
There was a problem hiding this comment.
I would go with #2. I think this is a great refactor which makes this code much clearer for the reader. I wouldn't expect you to have to boil the ocean to make a substantive improvement like this.
|
Thanks @douglashall. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Friday, October 13, 2017. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
3 participants
No description provided.