Skip to content
This repository has been archived by the owner on Jul 18, 2024. It is now read-only.

feat: upgrade mathjax from v2 to v3 #352

Conversation

navinkarkera
Copy link
Contributor

See openedx/edx-platform#33555 for details.

@navinkarkera navinkarkera requested a review from a team as a code owner October 26, 2023 15:41
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 26, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Oct 26, 2023

Thanks for the pull request, @navinkarkera! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (88e1234) 52.41% compared to head (c1b485a) 52.41%.
Report is 9 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #352   +/-   ##
=======================================
  Coverage   52.41%   52.41%           
=======================================
  Files          75       75           
  Lines        1986     1986           
  Branches      359      359           
=======================================
  Hits         1041     1041           
  Misses        912      912           
  Partials       33       33           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@itsjeyd itsjeyd added the core contributor PR author is a Core Contributor (who may or may not have write access to this repo). label Nov 2, 2023
@itsjeyd
Copy link

itsjeyd commented Nov 2, 2023

Hey @navinkarkera, is this PR ready for review?

@itsjeyd itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Nov 2, 2023
@navinkarkera
Copy link
Contributor Author

@itsjeyd Yes.

@itsjeyd itsjeyd added waiting for eng review PR is ready for review. Review and merge it, or suggest changes. and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Nov 2, 2023
@itsjeyd
Copy link

itsjeyd commented Nov 2, 2023

@jristau1984 This PR is ready for engineering review by TNL.

@tecoholic
Copy link

@navinkarkera Kindly add "jira:2U" label to this, if you have access.

cc: @itsjeyd just in case Navin didn't have the access on this repo.

@jristau1984 jristau1984 added the jira:2u We want an issue in the 2U Jira instance label Nov 9, 2023
@openedx-webhooks
Copy link

I've created issue TNL-11195 in the private 2U Jira.

@itsjeyd
Copy link

itsjeyd commented Nov 23, 2023

Hey @jristau1984, do you have any updates about when we can expect TNL to review this PR?

@itsjeyd itsjeyd requested a review from jristau1984 December 7, 2023 13:45
@jristau1984
Copy link
Contributor

@itsjeyd, @wittjeff has offered feedback that the impact of this change needs to be handled in a much more conservative way than just upgrading. Due to that, it has caused this PR to stall.

@itsjeyd
Copy link

itsjeyd commented Dec 14, 2023

@jristau1984 Thanks for the update.

... this change needs to be handled in a much more conservative way than just upgrading.

To understand better, is that work happening via another PR? Or would @navinkarkera be expected to address @wittjeff's concerns in the context of this PR (and if so where can he find the feedback)?

@itsjeyd itsjeyd removed the waiting for eng review PR is ready for review. Review and merge it, or suggest changes. label Dec 14, 2023
@navinkarkera
Copy link
Contributor Author

if so where can he find the feedback?

@itsjeyd We have feedback from Jeff in edx-platform PR: openedx/edx-platform#33555

@itsjeyd
Copy link

itsjeyd commented Dec 21, 2023

@navinkarkera OK I see. I'll keep this PR in the Blocked status, then. Let me know when you're ready to resume the work here.

CC @jristau1984

@itsjeyd itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Dec 21, 2023
@itsjeyd
Copy link

itsjeyd commented Jan 25, 2024

Hey @navinkarkera, do you have any updates on when this PR might get unblocked?

@itsjeyd
Copy link

itsjeyd commented Feb 2, 2024

@navinkarkera Friendly ping about my comment above.

@navinkarkera
Copy link
Contributor Author

@itsjeyd Apologies for missing your ping. This is blocked on openedx/edx-platform#33555 and we don't have a go ahead from 2U yet. Once course outline MFE project is completed, we might be able to get this across if 2U agrees.

@itsjeyd
Copy link

itsjeyd commented Feb 8, 2024

@navinkarkera Sounds good, thanks for the update!

@itsjeyd itsjeyd removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Feb 16, 2024
@bradenmacdonald
Copy link
Contributor

Hey folks, we are planning to deprecate frontend-app-library-authoring and roll all related library authoring functionality into frontend-app-course-authoring. I also don't want any mathjax-related code to appear in the MFEs at all. Each individual XBlock that uses MathJax should provide its own MathJax environment. So I believe this PR can be closed.

@navinkarkera
Copy link
Contributor Author

@bradenmacdonald Makes sense, I'll close the PR.

@openedx-webhooks
Copy link

@navinkarkera Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core contributor PR author is a Core Contributor (who may or may not have write access to this repo). jira:2u We want an issue in the 2U Jira instance open-source-contribution PR author is not from Axim or 2U
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants