Skip to content
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

DEPR-48 Remove all references to deprecated CourseTalk widget #1921

Merged
merged 1 commit into from
Mar 4, 2021

Conversation

sarina
Copy link
Contributor

@sarina sarina commented Mar 1, 2021

See https://github.com/edx/edx-platform/pull/25571

No doc ticket

Remove references to CourseTalk widget which was deleted as part of DEPR-48. See the above PR for more detail.

Reviewers

Possible roles follow. The PR submitter checks the boxes after each reviewer finishes and gives 👍.

  • Subject matter expert: @marcotuts
  • Subject matter expert:
  • Doc team review (sanity check, copy edit, or dev edit?): @edx/doc
  • Product review:
  • Partner support:
  • PM review:

FYI: Tag anyone else who might be interested in this PR here.

Testing

  • Ran ./run_tests.sh without warnings or errors

HTML Version (optional)

  • Build an RTD draft for your branch and add a link here

Sandbox (optional)

  • Point to or build a sandbox for the software change and add a link here

Post-review

  • Add a comment with the description of this change or link this PR to the next release notes task.
  • Squash commits

@sarina sarina changed the title feat!: Remove all references to deprecated CourseTalk widget DEPR-48 Remove all references to deprecated CourseTalk widget Mar 1, 2021
@sarina
Copy link
Contributor Author

sarina commented Mar 2, 2021

@nedbat I'm not sure how to read the failing test report - any advice?

@nedbat
Copy link
Contributor

nedbat commented Mar 2, 2021

The check output is verbose. Because it ended with TOTAL SPHINX WARNINGS: 1, you can search for "WARNINGS: 1" to find the book that had the warning, then scroll up to:

https://github.com/edx/edx-documentation/pull/1921/checks?check_run_id=2008093477#step:6:1111

/home/runner/work/edx-documentation/edx-documentation/en_us/open_edx_release_notes/source/eucalyptus.rst:110:
WARNING: undefined label: installation:add coursetalk (if the link has no caption the label must precede a section header)

A tricky case: the thing you are deleting is referred to by old release notes.

@sarina
Copy link
Contributor Author

sarina commented Mar 2, 2021

Huh.... removing references from old release notes seems incredibly wrong, I'll try to see what I can to to preserve some of the information, I suppose?

@sarina
Copy link
Contributor Author

sarina commented Mar 2, 2021

Hrm. @nedbat would it be better to keep the documentation and edit it to indicate it's deprecated, or simply remove the reference from the 2016-0411 release notes?

@nedbat
Copy link
Contributor

nedbat commented Mar 4, 2021

Ugh, I guess the ideal would be that the Eucalyptus release notes are published from the eucalyptus branch, so that master could advance without worry.

But we don't have that. I guess the best thing now is to leave the coursetalk page with a warning at the top that it's for historical use only. Maybe it can be removed from the index of the book so it's only found if linked to?

@sarina sarina force-pushed the sarina/DEPR-48 branch 2 times, most recently from 00645ef to 1854e89 Compare March 4, 2021 13:16
@sarina
Copy link
Contributor Author

sarina commented Mar 4, 2021

@nedbat @marcotuts this is ready to review, I'll squash commits once review is done

@sarina sarina merged commit 51d96bf into master Mar 4, 2021
@sarina sarina deleted the sarina/DEPR-48 branch March 4, 2021 17:21
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.

2 participants