Skip to content

Conversation

@ttak-apphelix
Copy link
Contributor

@ttak-apphelix ttak-apphelix commented Aug 1, 2025

Description

This PR replaces brand-edx.org with openedx theme and adds it to the package file indirectly via aliasing.

Supporting information

Issue Link: edx/edx-arch-experiments#943

Link to the PR which is associated with the same issue
edx/configuration#230

@ttak-apphelix ttak-apphelix marked this pull request as ready for review August 5, 2025 09:51
Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I'm concerned about the size of the package-lock.json change-- that seems way to high of a line count for such a small package.json change. What command did you run to regenerate package-lock.json?

@robrap
Copy link
Contributor

robrap commented Aug 11, 2025

@ttak-apphelix: Next steps:

  1. Work with @kdmccormick to see if you can get a simpler change set, regarding:

Thanks for the PR. I'm concerned about the size of the package-lock.json change-- that seems way to high of a line count for such a small package.json change. What command did you run to regenerate package-lock.json?

  1. Did we already put QE resources toward testing changes? If not, let's do that, but after seeing if there is a simplification.
  2. Once all this had been done, we'll continue to see if we can get 2U reviews. Thanks.

@ttak-apphelix
Copy link
Contributor Author

ttak-apphelix commented Aug 13, 2025

Thanks for the PR. I'm concerned about the size of the package-lock.json change-- that seems way too high of a line count for such a small package.json change. What command did you run to regenerate package-lock.json?

@kdmccormick I was deleting node_module folder but by mistake I added package-lock.json file also. I have corrected that change and now it shows minimal changes in package-lock.json file

@ttak-apphelix
Copy link
Contributor Author

@ttak-apphelix: Next steps:

  1. Work with @kdmccormick to see if you can get a simpler change set, regarding:

Thanks for the PR. I'm concerned about the size of the package-lock.json change-- that seems way to high of a line count for such a small package.json change. What command did you run to regenerate package-lock.json?

  1. Did we already put QE resources toward testing changes? If not, let's do that, but after seeing if there is a simplification.
  2. Once all this had been done, we'll continue to see if we can get 2U reviews. Thanks.

@robrap

  1. I have corrected the changes and they are minimal in package-lock.json.
  2. I have tested myself and ran this branch on sandbox and everything is working as it was earlier.

Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

Thanks, the code looks good to me. I haven't tested, so please make sure some basic smoke-testing has been done before merging.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

salman2013 pushed a commit to salman2013/edx-platform that referenced this pull request Sep 10, 2025
…nedx (openedx#37105)

* chore: update @edx/brand dependency to new package @openedx/brand-openedx

* Revert package-lock.json

* fix: updated package-lock.json
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