Skip to content

Conversation

@cahrens
Copy link

@cahrens cahrens commented Nov 26, 2013

STUD-870

@auraz and @polesye Please review this PR (including checking it out and testing the video transcript workflow manually). All tests are passing, but I don't know how these functions work well enough to manually test myself.

I did consider leaving some of the variable names as "component_id" instead of changing to "component_locator" (to reduce the number of changes in the PR). However, that was causing me confusion when trying to understand if I changed everything that I needed to.

I have not done anything to the form of the URLs. Since changing video transcript URLs to be RESTful is not necessary for unblocking the database re-architecture epic, we have decided to postpone that work.

Copy link
Author

Choose a reason for hiding this comment

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

This was missing a var declaration.

@polesye
Copy link
Contributor

polesye commented Dec 2, 2013

Looks good for me 👍

@auraz
Copy link
Contributor

auraz commented Dec 2, 2013

👍

@valera-rozuvan
Copy link
Contributor

Another issue that this PR fixes: BLD-554.

cahrens pushed a commit that referenced this pull request Dec 4, 2013
Change video transcripts to use locators instead of locations.
@cahrens cahrens merged commit 2682a14 into master Dec 4, 2013
@cahrens cahrens deleted the christina/transcripts branch December 4, 2013 14:41
jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Mar 24, 2017
* Add font-style normal. openedx#1790

* Mod error message when disable to register student-self with input status. openedx#1787

* Mod biz-menu style height to min-height. openedx#1793

* Fix review(TYPO).
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.

5 participants