Upstreaming, forks, plugins & themes

Following several discussions lately, I wanted to bring back to our attention one important principle of how we work: we aim to contribute all our work to the Open edX project.

This is one of the main reasons that OpenCraft exist, to align the interests of clients needing customization with the open source project, so that both benefit from each other. By contributing the work, the clients get to share the maintenance load of their features with the rest of the project, while the rest of the project gains new features. And for us in the middle, we don’t end up caught up in the vicious circle of pushing quick changes that are a nightmare to maintain on the long term, especially during upgrades. It’s a guarantee we offer to our clients, to the project, and to the rest of the team who will have to deal with the code.

Core vs plugins

That doesn’t mean that everything needs to be upstreamed to the core, but every development we do should end up into one of these two categories:

  • Upstream PR: for core features
  • Plugin: for core and non-core features that fit in a plugin (note that the extension points might not exist, and would then also require an upstream PR to add them).

Also note that even in the case of a plugin, we should still contribute it to the project: post product proposals, ADR, and if there is interest for having it in the standard distribution, still get it included there. This is important, as more and more features will be developed as plugins - we should still do the effort of “upstreaming” those.

This isn’t specific to Open edX - changes to other open source projects and libraries follow the same approach.

One exception might be when we need to modify client code which might be proprietary in the first place - but we aim to keep those to a minimum, and will only rarely accept to do this, when it’s necessary for the rest of our work.

The big NO’s!

Specifically, the following approaches are banned:

  • Making a change in a fork of the platform without upstreaming that change
  • Making a logic/code change in a theme
  • Making the change in a plugin that we don’t attempt to contribute to the project

We know from experience that some of these options are sometimes very tempting, especially when the clients don’t have much budget, and are pressuring for small quick fixes. We (and some clients) have however been bitten hard in the past by taking that road, and we owe it to them to remind them that, on the long term, this will actually end up being more costly, in particular because of the increased maintenance cost. Making features good and generic enough to be useful in the project takes more time and effort upfront, but it’s much better than ending up spending that time keeping a hacky change afloat forever.

Getting help

If you have encountered that situation, or if you do in the future, don’t hesitate to bring it up in the forum. It can feel like being stuck between a rock and a hard place, but you don’t have to deal with it alone. We have many people in the team who went through it numerous times, and there often ways to find a good solution.

And with the core contributors in the team, there is likely someone in the team that can help with the contribution part, or the upstreaming strategy. If that’s your case now, don’t hesitate to bring it up now. :slight_smile:

Proposal: core contributor reviews of discoveries

Btw, since often these problems start at the estimation phase, by not provisioning enough time to properly generalize and contribute the features, it could now make sense to get technical discoveries reviewed by a core contributor - one that has the closest expertise for the subject at hand?

[ Ticket to log time answering ] (or a given project ticket/epic if you need to discuss a specific case)

7 Likes

@antoviaque A few times, I or others have made an offer where we explicily tell the client that we have two ways of working on a task, one of which is an ad-hoc solution not geared to be upstreamed, where the long-term maintenance costs will be much higher. We explicitly say we don’t recommend it, but have offered it as an option with warnings.

What I’m hearing here is that it is not to be an option at all, and that if we can’t upstream it, we just won’t do it, unless we’re dealing with the client’s own, pre-existing proprietary code they aren’t open sourcing. Is that correct?

1 Like

@antoviaque Thanks for the valuable reminder.

:heart_eyes:

I’d like to add that it would be helpful to involve a “product person” at an early stage as well. Someone like me or @cassie could present the proposal to the Product Working Group to help generalize the features from a UX/UI perspective. This will enhance the likelihood of the feature being useful to the broader Community or even being accepted into Core.

2 Likes

I totally second this @Ali. Reviewing the Graded Discussions proposal reviews this afternoon has proven your point! :+1:

@Fox That is correct - we definitely should not volunteer it as an option, and when a client requests it, explain why this is a bad idea for everyone. A good car mechanic would not suggest or accept to make fixes that he knows will eventually damage the car or cost more to the client on the long term - we shouldn’t either. Especially now that there is a focus on pluggability, we should at least fully try the proper route.

Nothing is fully absolute so there can always be edge cases (for example when clients come with an existing large backlog of code drift), but we should be really careful in those cases. To keep it simple, let’s say that if neither @braden or I approved an exception to the cases I was listing as banned above, we should not do it, or offer it.

1 Like

@antoviaque Are custom footers for MFEs ok?

@antoviaque Since you’re on vacation, and I don’t want to delay the decision, since the client is asking about it now, I will make the decision based on the current arguments, and if you disagree, we can discuss it once you’re back.

From the discussions I’ve had with other OC members, custom headers and footers for MFEs should be ok, since they are very similar to the custom theme. It’s expected that clients would like to modify them, and previously we would do it through the theme, and we can’t do it anymore in MFEs.

My understanding, after talking with @kshitij, is that upstream realized that making header and footer so customizable that it would fit everyone’s needs would be challenging, so instead it was originally designed in a way that would require you to roll your own, e.g. by forking the upstream version and modifying it to suit your needs.

Since forking requires a lot of maintenance, especially in our case, where we have more than one client, we looked into upstreaming some of the code that would allow customizing the footer in a way that is frequently requested, e.g. adding custom links to the footer. See the following links: BB-7755, upstream PR, ADR PR.

The above has been rejected for more than one reason, however from the comments it became apparent that the upstream maintainers recognize the existing issue with the MFEs, which is that they are not easy to customizing without forking, and at the same time adding too many parameters would make the upstream code difficult to maintain. Also they hinted at the fact that a new way would be added to customize existing MFEs:

I don’t think we need to make a decision for the whole project, here and now. (Spoiler alert, we’ll soon be having a frontend-wide extensibility discussion, during which this issue will be brought up.) But this issue is pretty symbolic. Do we add more configuration? Do we wait for a more comprehensive customization framework? Both?

To my best knowledge, this method is in “experimental” stage, and is referred to as “slots”:

However, the slots are still experimental, from what I could tell, and are not part of any releases. I checked a few repos, and the commits that add slots are in the master branched, so they are not included in the Redwood.

So we are kind of in a gray area. We don’t know if the slots will stick, and I’m also not confident without testing, if what they clients are asking us to do can be achieved using slots. Also, it seems like it would require storing custom code in grove cluster repositories as tutor plugins (not great to maintain on a shared cluster, if many clients request customizations).

So I would say, that since the header and footer have were intended to be forked, and the alternative solution is still not stable and not included in the current release, I would go for creating a fork for a client, and then if slots are included in the next release (or added to redwood.2 or redwood.3), then we would remove the fork and port it to slots.

cc. @kshitij since we had this discussion IRL, if you have anything to add, please do.