Upstreaming, forks, plugins & themes

Discussions - Public
1

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
2

@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
3

@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
4

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

5

@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
6

@antoviaque Are custom footers for MFEs ok?

7

@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.

8

The impression I got from the conference is that slots are the way to go and will definitely stick. What seems to be up for debate is some of the terminology and configuration mechanics with the current system having some redundancies (e.g. why have a hide mechanism when you have a replacement mechanism that can replace with nothing).

I have a task I’ve created just recently to go through current areas of drift and see what slots are needed to get rid of the drift.

We would not necessarily need to store any custom code in grove clusters repos as tutor plugins, most of the code can be kept in a separate repo and installed and loaded from there. The exact mechanism for this needs to be ironed out but it’s new so we can look into it as the need arises.

9

You guys should watch the “Frontend Pluggability” workshops we presented at the conference :slight_smile:

Slots and plugins are here to stay, and we worked hard to ensure they’re in the Redwood release. However, as of yet there are not many slots defined so if you want to customize something with a plugin, you’ll likely have to create a new slot first (and upstream that). We showed how to do that in the second half of the workshop (slides).

As @kshitij mentioned, the configuration format is likely to change but the overall approach of defining “slots” in MFEs and then customizing those with plugins is here to stay.

4 Likes
10

@braden @kshitij Thank you for replying while I was away - going for slots to implement this makes sense to me, that sounds right. :+1: @maxim What have you opted for at the end?

We also need to remember to contribute the slots/plugins/xblock/etc work too, as much as possible, even if it’s not part of the core. We aim to develop it in a way that favors reuse by others in the community. This helps to share the maintenance burden, and thus reducing it on the long term.

11

@antoviaque the client found a work around for what they wanted. and which also worked better for their use case, so we didn’t make any forks.

1 Like
12

Based on this forum post on Open edX forum, the method of installing a custom footer through npm is getting deprecated in favor of slots.