search

openedx / open-edx-proposals

Proposals for Open edX architecture, best practices and processes
http://open-edx-proposals.readthedocs.io/
Other
44 stars 32 forks source link

docs: adding an ADR to OEP-65 about a unified platform repository #598

Closed davidjoy closed 1 month ago

davidjoy commented 3 months ago

This ADR describes a decision to create a unified platform library, combining frontend-build, frontend-platform, frontend-plugin-framework, frontend-component-header, frontend-component-footer, and eslint-config. It will also include the ‘shell’ application for the new architecture proposed in OEP-65.

openedx-webhooks commented 3 months ago

Thanks for the pull request, @davidjoy!

What's next?

Please work through the following steps to get your changes ready for engineering review:

:radio_button: Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

:radio_button: Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

:radio_button: Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

:radio_button: Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @sarina. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

:bulb: As a result it may take up to several weeks or months to complete a review and merge your PR.

itsjeyd commented 2 months ago

Hey @davidjoy and @bradenmacdonald! OSPR management check-in:

What are the next steps here? Will the changes need review from more people before we can consider them ready for merge?

sarina commented 2 months ago

I requested review from @brian-smith-tcril who is on holiday through mid next week, as well as @arbrandes. @bradenmacdonald is there anyone else we should tag?

bradenmacdonald commented 2 months ago

@sarina Not sure if he'll want to review this or not but I'd like to make sure that any remaining concerns from @regisb have been addressed.

regisb commented 2 months ago

I have some concerns about this PR, but not all of them need be addressed right now. (For instance, I'm not convinced that bundling 5 dependencies in one will simplify dependency management. Sure, there will be just one package for which versions will need to be in sync, but breaking changes will also happen five times more often.)

My only remaining issue is with the planning. Specifically, I'd like to have a mechanism in place for dependency synchronisation ASAP rather than at the end of the implementation of OEP-65. The completion date for OEP-65 is expected after the Sumac release. But having consistent versions for the header, footer and other libraries in all MFEs is something that would be extremely useful right now. Since we will have to set this up anyway, why not start there?

davidjoy commented 2 months ago

Simplifying the dependency graph between frontend-build, frontend-platform, frontend-component-header, frontend-component-footer, frontend-plugin-framework, (as well as frontend-slot-footer, eslint-config, and typescript-config, it turns out) will remove a significant number of 'edges' between them, and between them and MFEs.

Any breaking changes internal to that graph are no longer breaking changes. With the addition of the shell, we further simplify the dependency tree between these repos and MFEs. We also reduce the API surface since many things MFEs needed to do, the shell will now do. This reduces the number of potential breaking changes even further. Yes, any that do occur will happen in this library, rather than spread across 5-8. Reading, understanding, and absorbing breaking changes from one change log will be easier than doing it across many.

This lowers dependency overhead, plain and simple. It doesn't, in and of itself, give us a better dependency management process. But frankly it seems like a blatantly 'better' place to be with respect to the actual dependency graph.

Since we will have to set this up anyway, why not start there?

If this is implying I should stop my work and go work on that instead, then I guess we can all talk about that with Axim, who I'm contracting with. That implies the short term tactical necessity of dealing with dependency management is so great that we should re-allocate folks working on strategic initiatives to work on it instead.

If it's not implying that this is up to me, personally, to drop everything and go work on it, then my work with this ADR and OEP-65 should continue in parallel, and this is an unrelated discussion to the decision in this ADR. We'd discussed finding volunteers to help dedicate some time to dependency management, and I'm more than happy to work with those folks.

itsjeyd commented 1 month ago

Hey @arbrandes and @brian-smith-tcril, a friendly reminder to check out this PR when you have a minute.

sarina commented 1 month ago

@davidjoy what's the status of this one?

davidjoy commented 1 month ago

@sarina Just need to respond to Brian's feedback above. If I can get another 👍🏻 from either @brian-smith-tcril or @arbrandes after that I'd consider this ready to merge.

openedx-webhooks commented 1 month ago

@davidjoy 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.