Skip to content

Remove unused profile name. - #11751

Merged
cahrens merged 1 commit into
masterfrom
christina/remove-profile-name
Mar 7, 2016
Merged

Remove unused profile name.#11751
cahrens merged 1 commit into
masterfrom
christina/remove-profile-name

Conversation

@cahrens

@cahrens cahrens commented Mar 4, 2016

Copy link
Copy Markdown

The profile name requires escaping when displayed. Proactively removing since it is not being used. Also discovered some dead code.

@@ -1,272 +0,0 @@
// structure stolen from http://briancray.com/posts/javascript-module-pattern

var CohortManager = (function ($) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this code was from the first version of the cohort manager (in the instructor dashboard). It was written, but this file was left behind.

As far as I can tell, this is the place where profile.name was displayed when users moved between cohorts or were added to a cohort.

@cahrens

cahrens commented Mar 4, 2016

Copy link
Copy Markdown
Author

@robrap and @dan-f can you please code review this cleanup? I'm tagging you @dan-f because you once edited course_groups/cohorts.js. :)

@dan-f

dan-f commented Mar 5, 2016

Copy link
Copy Markdown
Contributor

👍 for deleting old JS!

The profile name requires escaping when displayed. Proactively removing since it is not being used. Also discovered some dead code.
@cahrens
cahrens force-pushed the christina/remove-profile-name branch from d8a6bfb to 2f3e3a5 Compare March 7, 2016 14:37
@cahrens

cahrens commented Mar 7, 2016

Copy link
Copy Markdown
Author

@robrap please review

@robrap

robrap commented Mar 7, 2016

Copy link
Copy Markdown
Contributor

👍 Thanks for the clean up.

cahrens pushed a commit that referenced this pull request Mar 7, 2016
@cahrens
cahrens merged commit e6fcfae into master Mar 7, 2016
@cahrens
cahrens deleted the christina/remove-profile-name branch March 7, 2016 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants