Skip to content

Upload transcript from basic tab #17632

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.

Sign up for GitHub

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

Jump to bottom
Merged
Qubad786 merged 1 commit into from
Mar 21, 2018
Merged

Upload transcript from basic tab #17632

Qubad786 merged 1 commit into from
Mar 21, 2018

Conversation

@Qubad786
Copy link
Contributor

@Qubad786 Qubad786 commented Mar 7, 2018 •
edited
Loading

@edx-status-bot
Copy link

edx-status-bot commented Mar 7, 2018

Your PR has finished running tests.

@Qubad786 Qubad786 force-pushed the mrehan/upload-transcript-basic-tab branch from 610dd81 to 46c7580 Compare March 14, 2018 08:19
@edx-status-bot
Copy link

edx-status-bot commented Mar 14, 2018

Your PR has finished running tests.

2 similar comments
@edx-status-bot
Copy link

edx-status-bot commented Mar 15, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 16, 2018

Your PR has finished running tests.

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 18, 2018

jenkins run bokchoy

@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

@Qubad786 Qubad786 force-pushed the mrehan/upload-transcript-basic-tab branch from f3ad0fa to 2fa91dd Compare March 18, 2018 15:04
@Qubad786 Qubad786 changed the base branch from master to transcripts-phase-2 March 18, 2018 15:07
@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

2 similar comments
@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 18, 2018

jenkins run all

@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

3 similar comments
@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 18, 2018

jenkins run all

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 18, 2018

@muhammad-ammar @mushtaqak this is ready for your review.

@edx-status-bot
Copy link

edx-status-bot commented Mar 18, 2018

Your PR has finished running tests.

2 similar comments
@edx-status-bot
Copy link

edx-status-bot commented Mar 19, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 19, 2018

Your PR has finished running tests.

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 19, 2018

jenkins run python

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 19, 2018

jenkins run lettuce

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 19, 2018

jenkins run bokchoy

@edx-status-bot
Copy link

edx-status-bot commented Mar 19, 2018

Your PR has finished running tests.

@Qubad786 Qubad786 force-pushed the transcripts-phase-2 branch from 11a943b to d4a2db4 Compare March 20, 2018 18:54
@Qubad786 Qubad786 force-pushed the mrehan/upload-transcript-basic-tab branch from 9b7cb7d to 4456340 Compare March 20, 2018 20:31
@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 20, 2018

@muhammad-ammar @irfanuddinahmad TODOs related to check_tanscripts are functional now.

@Qubad786
Copy link
Contributor Author

Qubad786 commented Mar 20, 2018

@irfanuddinahmad Its expected to be merged in phase 2 feature PR: https://github.com/edx/edx-platform/pull/17718

muhammad-ammar
muhammad-ammar approved these changes Mar 20, 2018
View reviewed changes
Copy link
Contributor

@muhammad-ammar muhammad-ammar left a comment

Choose a reason for hiding this comment

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

@Qubad786 LGTM. Left some minor comments.

cms/djangoapps/contentstore/views/tests/test_transcripts.py Outdated
Copy link
Contributor

@muhammad-ammar muhammad-ammar Mar 20, 2018

Choose a reason for hiding this comment

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

why not BOM? why BOM_UTF8?

Copy link
Contributor Author

@Qubad786 Qubad786 Mar 21, 2018

Choose a reason for hiding this comment

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

@muhammad-ammar BOM is an alias for BOM_UTF16: https://docs.python.org/2/library/codecs.html

cms/djangoapps/contentstore/views/tests/test_transcripts.py Outdated
Copy link
Contributor

@muhammad-ammar muhammad-ammar Mar 20, 2018

Choose a reason for hiding this comment

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

I think docstring needs to be updated

cms/djangoapps/contentstore/views/tests/test_transcripts.py Outdated
Copy link
Contributor

@muhammad-ammar muhammad-ammar Mar 20, 2018

Choose a reason for hiding this comment

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

vertical or video?

@edx-status-bot
Copy link

edx-status-bot commented Mar 20, 2018

Your PR has finished running tests.

1 similar comment
@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

irfanuddinahmad
irfanuddinahmad approved these changes Mar 21, 2018
View reviewed changes
Copy link
Contributor

@irfanuddinahmad irfanuddinahmad left a comment

Choose a reason for hiding this comment

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

Looks good ...

@Qubad786 Qubad786 force-pushed the mrehan/upload-transcript-basic-tab branch from 3da3920 to 0709e3c Compare March 21, 2018 07:36
@Qubad786 Qubad786 merged commit 1680cbf into transcripts-phase-2 Mar 21, 2018
@Qubad786 Qubad786 deleted the mrehan/upload-transcript-basic-tab branch March 21, 2018 07:38
@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

7 similar comments
@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

@edx-status-bot
Copy link

edx-status-bot commented Mar 21, 2018

Your PR has finished running tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@irfanuddinahmad irfanuddinahmad irfanuddinahmad approved these changes

+1 more reviewer

@muhammad-ammar muhammad-ammar muhammad-ammar approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Qubad786 @edx-status-bot @muhammad-ammar @irfanuddinahmad