-
Notifications
You must be signed in to change notification settings - Fork 48
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
chore: Update to the new version of paragon in the new scope. #1930
Conversation
9ac928d
to
1098271
Compare
@openedx/enterprise-titans this is a fairly major version bump in the paragon version. Let me know if there are any specific tests that I should perform above the basic CI to verify this changes. |
@feanil Luckily no React components from Paragon are used within this repo. That said, there are some features like the https://courses.edx.org/enterprise/select/active page that rely on some of the class names from Paragon/Bootstrap (e.g., ![]() The changes made in this PR that first introduced Given this, these changes otherwise feels fairly low risk to me, but some flows like the above one should likely be verified on stage/prod with a quick run through to ensure no style regressions. I'm not sure how familiar you are with setting up / testing Enterprise flows, so I'd be happy to help you with a quick manual run through of some common Enterprise user flows that depend on |
@adamstankiewicz I don't have access to your stage systems to verify these changes there. Is it possible to set these up in a pre-stage environment like devstack or tutor? |
@adamstankiewicz any update on this, do you have docs on how to setup the relevant views in a dev environment or can you do the verification on stage for this? |
Part of openedx/axim-engineering#23 This replaces the `@edx/paragon` packag to point to the `paragon` package at the `openedx` scope(`@openedx/paragon`). Imports have been updated to use the same locations in the new package.
1098271
to
19cecd3
Compare
@adamstankiewicz I was able to get this running locally and verify that there weren't any paragon CSS related errors on the enterprise pages: Before upgradeAfter UpgradeI think this change is ready to go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving based on @adamstankiewicz's comments and @feanil's testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@feanil thanks for verifying with the screenshot.
If this intends to be released to PyPi as a new version such that it can be upgraded in edx-platform, the PR should include a CHANGELOG entry and an update to the version specified in the init.py
file.
@feanil @arbrandes Do you plan on getting these changes upgraded into edx-platform
or are you planning on just merging the PR?
@adamstankiewicz yes, I'll bump the version, add the changelog entry and get this landed in edx-platform. |
Update the changelog and bump the version.
Part of openedx/axim-engineering#23
This replaces the
@edx/paragon
packag to point to theparagon
package atthe
openedx
scope(@openedx/paragon
). Imports have been updated to use thesame locations in the new package.