-
Notifications
You must be signed in to change notification settings - Fork 4
Bumps frontend-build@^14, frontend-platform@^8 #16
Bumps frontend-build@^14, frontend-platform@^8 #16
Conversation
to support upgrading @edx/frontend-plaform Command used: npm install --saveDev "@openedx/frontend-build"@">= 14.0.0"
Command used: npm install --saveDev "@edx/frontend-platform"@"<9.0"
Thanks for the pull request, @pomegranited! 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:
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. |
…ai-translations to support frontend-build v14 Depends on: openedx/frontend-lib-content-components#475 openedx-unsupported/frontend-component-ai-translations#16 Commands used: npm install "git+https://github.com/open-craft/frontend-lib-content-components.git#jill/bilalqamar95/jest-v29-upgrade" npm install "git+https://github.com/open-craft/frontend-component-ai-translations.git#jill/frontend-platform-v8"
…ai-translations to support frontend-build v14 Depends on: openedx/frontend-lib-content-components#475 openedx-unsupported/frontend-component-ai-translations#16 Commands used: npm install "git+https://github.com/open-craft/frontend-lib-content-components.git#jill/bilalqamar95/jest-v29-upgrade" npm install "git+https://github.com/open-craft/frontend-component-ai-translations.git#jill/frontend-platform-v8"
Hi @mphilbrick211 , I don't know if there are any CCs with merge rights on this repo? But it's blocking us merging openedx/frontend-app-authoring#1052, so it's getting urgent. Could someone from Axim help out? |
Hi @pomegranited! I'll check for you. |
@pomegranited, I don't think anybody outside 2U is set up to properly review or test this. I'm actually surprised this repository is even a thing, let alone a dependency in a core repo. I'm going to argue for its deprecation/removal. In the meantime, it seems @hajorg is a major contributor here. Could you take a look, please? |
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.
lgtm. Left a question.
@@ -8,6 +8,7 @@ | |||
}, | |||
"scripts": { | |||
"build": "make build", |
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.
What is the purpose of this prepare
script? It's identical to the build
one.
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.
Hi @BrandonHBodine -- this change is optional, but it lets us reference unmerged branches when working on dependency upgrades like this.
Since npm v5.0.0 (ref):
git dependencies with
prepare
scripts will have theirdevDependencies
installed, andnpm install
run in their directory before being packed.
Hey @arbrandes, I think this was our answer to getting something in before the introduction of the plugin framework. It currently is a pain on the 2u side as well. Do you know where the discussions for a adding a plugin slot to the course authoring MFE take place? |
@BrandonHBodine, thanks for reviewing!
There are a few places to start an conversation about that. The #wg-frontend channel in the Open edX Slack is a good one, but the FWG meeting every other Thursday also works. So would the forum, for that matter. But the fastest way is usually to open a PR - in this case to frontend-app-course-authoring - where we can iterate on a solution quickly. |
Thank you for your review @BrandonHBodine ! Could this be merged and tagged so we can unblock openedx/frontend-app-authoring#1052? CC @arbrandes |
@pomegranited 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Thanks @BrandonHBodine! |
Upgrades dependencies to support parallel upgrades to frontend-app-course-authoring.
Supporting information
openedx/frontend-app-authoring#1034
Private-ref: FAL-3750
Author Notes & Concerns