-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Improve documentation use of common and openedx directories #33139
Comments
Thanks for raising this @robrap! Some further thoughts/questions for consideration:
|
I think that at the time there was a substantial difference in testing time between the two, which made it much nicer to work in lib when this was split out. So much has changed since then, that it is likely that this no longer an issue, and merging would simplify. FYI: @feanil @kdmccormick: Regarding this ticket in general, not just this comment. |
+1 to this ticket 👍🏻 @robrap @bradenmacdonald Some context:
The reason the openedx dir exists was a desire to have a top-level import for all openedx code. The idea is that eventually we'd nest everything under it, even across repositories, eg This clearly didn't pan out, so now we just have two folders for common code. I support picking one and making it clear that no new code should be added to the other one.
The openedx/features folder actually came three years later without much explanation. Based on what I recall @nasthagiri telling me, I believe you are right that the impetus for it was to better co-locate Python modules with their supporting assets. Based on that, I think we can abandon this structure as we migrate those features to MFEs.
Great question. I think that apps which are clearly only used by lms or cms should be moved to the corresponding directory. (That said, as @ormsbee has pointed out, most content-related apps don't fall in this category, as they have both author-facing features and learning-facing features. For example, consider a "scheduling" app: authors need to set schedules in Studio, and then instructors and learners need to be see the effects of those schedules in Courseware. What's the right architecture there? Does the entire app belong in the common dir? Or should the code be split into three parts (authoring, learning, shared)? I think the ideal pattern would be something like XBlock: there's be a single "scheduling" app which would coherently hook itself in to cms and lms, but without creating coupling between cms and lms.)
A couple years ago I worked to make sure that edx-platform's folder structure matched its import paths. That is, if you see an import like The folder at common/lib/xmodule/xmodule was configured to be imported as Looking to the ~5-year future, I think we could eventually dissolve this directory by extracting all the XBlocks, and then moving Modulestore to the cms directory. This would require severing the lms->Modulstore dependency, which is a long-term goal of the Learning Core.
I agree with Robert, we can merge these. These are a relic from the time when we imagined that XBlocks wouldn't depend on Django. That ship has sailed. There's a wiki page or ADR for this somewhere.
Based on my experience with the import work, reorganization is totally doable and carries huge benefit. That said, it's careful work and folks are generally too busy or too nervous. The toughest parts I found are:
I believe @feanil is considering this sort of work for the new Aximprovements team. |
Thanks @kdmccormick. I think the quicker we can update the docs (followed by linting) for what we know, the better. It will make things more clear, and will keep any future migration from growing. |
When someone new to the platform (or not new to the platform) is trying to determine where a new app should be created, it is not very clear. This is made more complex due to the lack of clarity around usage of the common and openedx directories.
Documentation Notes/Questions:
Usage Questions:
Other notes:
The text was updated successfully, but these errors were encountered: