Skip to content
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

OEP-57: Core Repositories #312

Closed
wants to merge 7 commits into from
Closed

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Feb 24, 2022

This OEP is a big ol' work-in-progress. Feel free to leave early feedback, but please keep in mind that it's still a rough outline. Lots of this stuff is my stream-of-consciousness. It's not yet representative of tCRIL's or anyone else's plans/opinions.

Fixes #295

@kdmccormick kdmccormick changed the title docs: OEP-XXXX: Core Repositories OEP-XXXX: Core Repositories Feb 24, 2022
@kdmccormick kdmccormick force-pushed the kdmccormick/core-repos branch 5 times, most recently from ace7ffa to f4b3dd9 Compare February 24, 2022 23:26
@kdmccormick kdmccormick changed the title OEP-XXXX: Core Repositories OEP-0057: Core Repositories Feb 24, 2022
Comment on lines +82 to +84
* review community code contributions,
* participate in community discussions related to the repository, and
* keep the repository compliant with the accepted Open edX Proposals (OEPs).
Copy link
Member Author

Choose a reason for hiding this comment

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

maybe I should drop just these requirements, and instead make sure they're in the list of maintenance responsibilities that OEP-0055 lays out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. I would say in the top-level bulled

it has assigned `maintainers`_ who continually meet the responsibilities defined in OEP-55


* This does not preclude other, smaller "cores" of Open edX (such as a *learning core*, *core utilities*, etc.) from existing within the Core repositories.

* **What repositories are in the Core?**
Copy link
Member Author

Choose a reason for hiding this comment

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

I realize that these criteria leave a gray area between repositories that must be in the core, but don't follow the standards for repositories that may be in the core.

I have some thoughts on how to reconcile this but am curious what others think.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I see what you're saying, but, if something is already a dependency of an Open edX release, it can't just be removed, so this feels to me like a grandfathered-in situation. I could see at line 91 adding something like:

We recognize that, at the time of writing, there exist repositories that must be in the core, but don't follow the standards for repositories that may be in the core. These repositories will be identified by the Core Contributor team, and go through a similar review process as defined above - with the express goal of finding active maintainers for each repo.

@kdmccormick kdmccormick changed the title OEP-0057: Core Repositories OEP-57: Core Repositories Feb 28, 2022
Copy link
Contributor

@sarina sarina left a comment

Choose a reason for hiding this comment

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

This is really coming along, great initial work @kdmccormick 🎉

:widths: 25 75

* - OEP-57
- :doc:`OEP-57 <oep-0057-core-repositories>`
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: According to the OEP spec, you should be putting proc in the document name (oep-0057-proc-core-repositories, see https://github.com/openedx/open-edx-proposals/tree/master/oeps/processes - all (except oep-1 lolol, and oep-54 where I also missed the notice) and https://open-edx-proposals.readthedocs.io/en/latest/processes/oep-0001.html#oep-header-preamble)

Comment on lines +82 to +84
* review community code contributions,
* participate in community discussions related to the repository, and
* keep the repository compliant with the accepted Open edX Proposals (OEPs).
Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. I would say in the top-level bulled

it has assigned `maintainers`_ who continually meet the responsibilities defined in OEP-55


* This does not preclude other, smaller "cores" of Open edX (such as a *learning core*, *core utilities*, etc.) from existing within the Core repositories.

* **What repositories are in the Core?**
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I see what you're saying, but, if something is already a dependency of an Open edX release, it can't just be removed, so this feels to me like a grandfathered-in situation. I could see at line 91 adding something like:

We recognize that, at the time of writing, there exist repositories that must be in the core, but don't follow the standards for repositories that may be in the core. These repositories will be identified by the Core Contributor team, and go through a similar review process as defined above - with the express goal of finding active maintainers for each repo.


* We declare that all access to write, configure, or otherwise administer Core repositories shall be granted through the `Core Contributor Program`_ and managed by its Program Administrators.

* We declare that the addition or removal of repositories from the Core should by reviewed by existing Core Contributors.
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as the process goes - we've been having trouble getting more than 30-40% of any given core contributors to participate in any CC program survey or standup we arrange. I think it would be good to, like the CC nomination process, define a minimum number of votes. The process should be public, where anyone can weigh in but only CCs get votes. Time-bounded is good. Would we specifically want maintainers to weigh in (who are already CCs)? Defining a rough template of how a repo "applies" for admission may be nice: repo name, purpose, core development team, who in the community is using it, plans for future development, and plan for maintainership all come to mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we need to just define a couple of general mechanisims for community voting and then we can refer to them in all the OEPs where we need that kind of input.

So far, we've got:

  • Minimum X +1s where a single -1 can shutdown the proposal.
  • Lazy Assent: Proposal is accepted by time X given no -1s by time X
  • It feels like there are more, but maybe just variants on these two?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having two voting mechanisms, "affirmative consent +N" defined as requiring N positive votes and no negatives, & "lazy assent: X days" as a time-bounded default acceptance of X days given no negatives, makes sense to me! What do you think would be the best way to define these? The absolute shortest OEP ever? 😁


However, from these many repositories, we wish to enumerate a smaller set which:

* is a good target for enhancement, documentation, and technical investment;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: before the bullets you use repositories... which, so your verbs at the beginning of bullets should be plural. eg, s/is a good target/are good targets/

Copy link
Member Author

Choose a reason for hiding this comment

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

I used singular verbs because a smaller set is the subject and a set is a single item. But if this still reads awkwardly then I can reword it.

Copy link
Contributor

@sarina sarina Mar 17, 2022

Choose a reason for hiding this comment

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

Ah, got it. Yeah for some reason it does read awkwardly (to me); I actually think then the problem is the which. "we wish to enumerate a smaller set which is a good target" vs "we wish to enumerate a smaller set that is a good target"

You can also ignore me if you disagree.

Copy link
Member

Choose a reason for hiding this comment

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

I agree but also agree it's not a blocker. Offered a suggestion if you want to fold it in, @kdmccormick


* The process for reviewing addition/removal of repos via the Core Contributor program isn't ready to be spun up yet or even well-defined. In the meantime, tCRIL has been making these judgement calls.
* 2U OCM and some of tCRIL retain historical access to many Core repositories outside of the Core Contributor program process. There is a plan to bring those folks under the Core Contributor umbrella, but it will take time.
* tCRIL is currently using a few public repositories in the openedx organization for day-to-day work, such as ``openedx/tcril-engineering``. Upon acceptance of this OEP, they would need to make a new GitHub organization for those repositories.
Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 migrating hundreds of GH issues will be a ton of fun!

.. _security policy: https://github.com/openedx/edx-platform/security/policy
.. _latest Open edX release: ./oep-0010-proc-openedx-releases.rst
.. _maintainers: ./oep-0055-project-maintainers.rst
.. _Core Contributor Program: ./oep-0054-core-contributors.rst
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: does this relative linking work in the RTD site? I've only seen explicit links in other OEPs.


* Resolving legacy processes and access policies

* The process for reviewing addition/removal of repos via the Core Contributor program isn't ready to be spun up yet or even well-defined. In the meantime, tCRIL has been making these judgement calls.
Copy link
Contributor

Choose a reason for hiding this comment

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

But won't this document define that process?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. I should have included the whole Considerations section in the PR description, not the OEP. It's meant to be a conversation with the folks previewing this OEP, not contents of the final OEP itself.

* Resolving legacy processes and access policies

* The process for reviewing addition/removal of repos via the Core Contributor program isn't ready to be spun up yet or even well-defined. In the meantime, tCRIL has been making these judgement calls.
* 2U OCM and some of tCRIL retain historical access to many Core repositories outside of the Core Contributor program process. There is a plan to bring those folks under the Core Contributor umbrella, but it will take time.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm hopeful all of tCRIL will have gone through the CC nomination process before this lands (so tCRIL folx - get that nomination thread going!)

oeps/processes/oep-0057-core-repositories.rst Show resolved Hide resolved
Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

Some early comments. Generally seems like it's heading in the right direction and we can always update as we try to apply the consequences.


* We declare that all access to write, configure, or otherwise administer Core repositories shall be granted through the `Core Contributor Program`_ and managed by its Program Administrators.

* We declare that the addition or removal of repositories from the Core should by reviewed by existing Core Contributors.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we need to just define a couple of general mechanisims for community voting and then we can refer to them in all the OEPs where we need that kind of input.

So far, we've got:

  • Minimum X +1s where a single -1 can shutdown the proposal.
  • Lazy Assent: Proposal is accepted by time X given no -1s by time X
  • It feels like there are more, but maybe just variants on these two?

oeps/processes/oep-0057-core-repositories.rst Show resolved Hide resolved
* Cleaning up the Core

* Many repositories in the openedx GitHub organization do not currently meet the Core criteria. We will need to review the organization contents, and for several repos, transfer them out or invest in them.
* Even so, the definition of the Core still captures a large number of repositories, probably larger than we want our "Core" to be. We could reduce it by loosening the package dependencies of many services to being optional instead of strictly required.
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it will be valuable to have a second org for the looser but still close to core repos. For example repos tha aren't required but that are defacto the way you do X if you don't have other opinions? Even if X plugs into an interface.

The idea of openedx-contrib as an org popped into my head as I was reading this so just mentioning it.

Copy link

Choose a reason for hiding this comment

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

I think it might be easier to use topics to tier the repos into different categories.

Copy link

Choose a reason for hiding this comment

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

I just feel like it would be painful to maintain sync with users, groups, settings and such across a second org. Or dealing with moving repos around between those orgs.

Copy link

Choose a reason for hiding this comment

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

We only use topics on a few repos now, and we use them in an inconsistent manner. But we could be more disciplined about it and make it something that folks could reasonably filter on, e.g. https://github.com/orgs/openedx/repositories?q=topic%3Afrontend&type=all&language=&sort=

Copy link
Member

Choose a reason for hiding this comment

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

I think it's worth holding off on a specific solution for this to see how things shake out with the maintainers and core changes. It might be that each contributing org maintains its own separate collection, or that the peripheral libraries fall clearly into guilds by topic. I'm just spitballing here but basically I think we should shift these core things and then see what the new emergent patterns are.

Copy link
Member

Choose a reason for hiding this comment

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

One small update on this idea--as 2U starts thinking about pilots for OEP-55 I'm realizing that one of our pain points is the way that our internal "ownership" spreadsheet scope is defined, which is basically "anything on the openedx or edx orgs". Arguably that definition will not scale, but I also can't think of a good alternative. Maybe org-based maintainership gets encoded as a tag? e.g. I can filter for all the repos currently maintained by 2U. That would allow us to continue to automate some of our ownership stuff without having to do weird things like duplicate the entire maintainer roster in our spreadsheet (which currently only lists teams internal to 2U).


* it is listed as a non-optional base dependency of a repository included in the latest Open edX release *and*

* it is *not* maintained as a third-party package that is unspecific to Open edX, such as Django or Python Requests.
Copy link

Choose a reason for hiding this comment

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

Nit: I think "unspecific" means vague/not well defined, and that what we'd want here is "not specific to Open edX". (Or maybe "nonspecific"? I never use that one.)

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* it is *not* maintained as a third-party package that is unspecific to Open edX, such as Django or Python Requests.
* it is *not* maintained as a third-party package separately from Open edX, such as Django or Python Requests.


* it is *not* maintained as a third-party package that is unspecific to Open edX, such as Django or Python Requests.

* In addition to the repositories that must be in the Core, a repository *may* be in the Core if:
Copy link

Choose a reason for hiding this comment

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

Instead of saying that there is a Core with some repos that must be in Core and some that may be in Core, with (Core == openedx GitHub org), would it be useful to define tiers of repos that will end up in the openedx GitHub org? Something like:

  • Core
  • Supporting
  • ...?

I realize that's somewhat at odds with the notion of aligning this language to "Core Contributors". I'm wondering if we have other kinds of information we want to convey in how we group these repositories–how actively they're developed, for instance. We've also maintained a number of forks of external open source projects over the years for upgrade or compatibility reasons, and I think it's confusing to lump that in as "Core".

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if directly aligning this with the "core contributors" language is a good idea if there isn't a 1:1 mapping. I like Dave's tiers idea, or even some qualitative schema like github topics. I think we would need to clean up and track our topic schema for sure, but it could provide a nice way to, say, group together a set of MFEs that are part of a consolidated effort to deprecate something (and are thus optional but collectively required if you remove the deprecated app). I'm thinking specifically of the instructor dash MFE project that 2U and OpenCraft will be collaborating on.

* We declare that the addition or removal of repositories from the Core should by reviewed by existing Core Contributors.

* TODO: Define a review process.

Copy link
Member

Choose a reason for hiding this comment

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

per Sarina's comment

Suggested change
We recognize that, at the time of writing, there exist repositories that must be in the core, but don't follow the standards for repositories that may be in the core. These repositories will be identified by the Core Contributor team, and go through a similar review process as defined above - with the express goal of finding active maintainers for each repo.


Many other repositories exist as part of the Open edX community. Some of these are forks of Core repositories, and others are upstream repositories in their own right. These various non-Core repositories are vital to different Open edX community members. Furthermore, the existence of non-Core repositories in general is important to the health of the Open edX project, which thrives on collaboration, experimentation, and extension.

However, from these many repositories, we wish to enumerate a smaller set which:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
However, from these many repositories, we wish to enumerate a smaller set which:
However, from these many repositories, we wish to enumerate a smaller set that:


* Ensuring the Core contains everything it should

* There are four repositories directly included in Maple that aren't in the openedx organization. We'll either need to remove them from the release or transfer them into openedx.
Copy link
Member

Choose a reason for hiding this comment

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

Where are we at on this for Nutmeg?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nutmeg master branches were created on Monday. There are four MFEs in the edx org included in Nutmeg, and two other repos that should probably be removed from the tagging process.


* In addition to the repositories that must be in the Core, a repository *may* be in the Core if:

* it is of interest to more than one entity the Open edX community (TODO: this needs better criteria); and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* it is of interest to more than one entity the Open edX community (TODO: this needs better criteria); and
* it is of interest to more than one entity in the Open edX community (TODO: this needs better criteria); and


* it is of interest to more than one entity the Open edX community (TODO: this needs better criteria); and

* it has assigned `maintainers`_ who strive to:
Copy link
Contributor

Choose a reason for hiding this comment

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

This only applies to code repos. We also have project repos (working group backlogs, etc).

TODO


Alternatives Considered
Copy link
Contributor

Choose a reason for hiding this comment

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

The big picture of this OEP as I understand it is:

  • Define criteria for a set of repos to be called Core, and
  • Make that set be the same as the openedx GitHub set.

I'm wondering if it's a good idea to join those two ideas together. As Dave commented, orgs imply access administrations, and as Sarina pointed out, there will be data to migrate if some repos have to be moved.

Couldn't we find other ways of clearly indicating what is Core? As written, this OEP will require making yet another organization (or maybe more than one?) The phrase "open edx" refers to a software platform, but also a project. Why shouldn't the openedx organization include repos that are in the project but not the platform?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might make the proposal more concrete if we could see a dry-run of what would have to move. What repos need to move into the openedx org, what repos would need to move out, and where they would go.

@feanil
Copy link
Contributor

feanil commented Aug 1, 2022

Just making a note on this OEP, that even though there has not been new activity on the PR, this is still being discussed and I believe there is still desire to get this written. It's currently just behind some other related work to roll-out maintainers(OEP-55) and so we don't have enough capacity to focus on this just yet.

@kdmccormick
Copy link
Member Author

Can confirm that there is still desire to get it written!

Since my last update here, I've taken a few steps back. Instead of starting with the question "what goes in the openedx organization?", I'm now working on answering "what is part of the Open edX project?". When we have an answer for the latter question, it'll be a lot easier to figure out tactical details such as GH administration.

For this new question, I'm starting by getting input from the Product WG. I recently shared a diagram with them: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/3499786241/Visualization+Brainstorm+-+Product+Core+and+Tech+Core. If you're following this PR, feel free to take a look, too.

@kdmccormick
Copy link
Member Author

Thanks for the review so far, everyone. I've read every comment and I'm factoring them in to my recent work on this. I'm going to close this PR and open a new one when I'm ready to start writing again.

In the mean time, I've been laying out a plan for what the next draft of the OEP(s) will look like: https://openedx.atlassian.net/wiki/spaces/COMM/pages/3511091214/OEP-57+Update+Game+Plan. Along with the diagram above, I invite everyone to take a look and comment.

@kdmccormick kdmccormick deleted the kdmccormick/core-repos branch August 15, 2022 19:34
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.

Write OEP answering "What code is part of the Open edX project?"
6 participants