search

openedx-unsupported / openedx-tech-radar

This repository serves as a version-able source of truth for the Open edX Tech Radar. It also has helper scripts for importing and exporting radars to/from the JSON-based format used here.
GNU General Public License v3.0
4 stars 3 forks source link

Ring placements in v1 of the Radar #4

Closed nasthagiri closed 2 years ago

nasthagiri commented 2 years ago

Thank you for bringing this over the line. Unlike Thoughtworks' Tech Radar that denotes movement of industry adoptions and trends, the Open edX Tech Radar is intended to be an evolving source of truth of community-wide technical decisions. Other organizations have also adopted this technique and I hope it will serve its long-term purpose for this community.

I have reviewed the position of the blips. Here is input for consideration:

Techniques

  1. BEES > Adopted -> Accepted: This architecture strategy was published a few months ago in the summer. I imagine there's more to do for it to be widely known and adopted in the vernacular, including ADRs, communications, etc.

  2. Conventional Commits > Adopted -> Accepted: Last I had checked, CCs were not yet fully adopted. There was an impending effort to add a linter to ensure you're at 100% usage. If you have done so, then yeah, keep it in adopted state. If you haven't, I'd move it to Accepted with the goal of it becoming Adopted soon.

  3. Micro-services > Clarify: Add a cross-reference to the Boundaries (DDD) blip to ensure people don't revert to creating too-small micro-services that are entity-based, exacerbating the current "distributed monolith" situation (where services are incorrectly dependent on each other). Consider rephrasing "small services" to "business and organization aligned smaller services".

  4. Synchronous signal handling (Django signals) > Adopted -> Accepted: Correct me if I'm wrong - the use of Django signals is still rudimentary in our adoption. More needs to be done in providing guidelines and possibly a framework for wider use and adoption. The upcoming Django-Events capability (from EduNext's Django Hooks efforts) will help establish this design pattern with structured APIs, versioning-strategy, etc.

  5. Standards-based Development > Provisional -> Accepted/Adopted: I am curious why this was put under Provisional. There are more than 3 example standards listed in the description that have already been implemented and widely adopted on the platform.

  6. Test-driven Development > Provisional -> ??: Are there any teams that are actually able to use TDD in their regular development? Perhaps they are able to in MFEs? The local test cycle in the Django apps today typically don't allow for TDD.

  7. Server side Templates > Clarify: Would you still want to use server-side templates for certain frontends? For example, Django admin pages? Legacy web-response pages? If so, may want to clarify the text to allow for server-side templates for certain frontends that are not worthwhile to support in MFEs.

Open edX

  1. celery-utils > Provisional -> Accepted: celery-utils has been around for several years now and used in multiple repositories. The implementation has been pretty stable for the last few years.

Technologies

  1. attrs > Adopted -> Accepted: Has there been much discussion about the learnings and tradeoffs with using attrs? My sense was that it's been starting to get traction, but not fully adopted yet with understanding on when to use and when not to.
sarina commented 2 years ago

@davidjoy - pinging for visibility. What are your thoughts?

I agree with: 1, 2, 3, 8

I don't know enough about why we placed the following where we did: 4, 5, 9 -> might want @robrap input on this, as he helped us placed the backend tech blips.

I definitely don't know if or how we use TDD. I try to, but am not contributing huge swaths of code.

robrap commented 2 years ago

This all looks good to me. Regarding TDD, would we lose anything by dropping it from the radar?

sarina commented 2 years ago

Regarding TDD, would we lose anything by dropping it from the radar?

I don't think so, frankly.

@davidjoy if you can weigh in with your thoughts, I can go ahead and make these changes to the radar.

davidjoy commented 2 years ago

I think I agree with it all too. Regarding server side templating, it might make sense to just mention that it continues to be normal and healthy for Django Admin, and that it exists in legacy frontends but the intention is to replatform those.

ormsbee commented 2 years ago

Synchronous signal handling (Django signals) > Adopted -> Accepted: Correct me if I'm wrong - the use of Django signals is still rudimentary in our adoption. More needs to be done in providing guidelines and possibly a framework for wider use and adoption. The upcoming Django-Events capability (from EduNext's Django Hooks efforts) will help establish this design pattern with structured APIs, versioning-strategy, etc.

I think it's fine to leave this in Adopted, FWIW. We have dozens of signals caught in probably hundreds of places. While we are refining these practices and building more a framework around them for openedx-events, the baseline usage of Django Signals has been normal practice in the codebase for years now. We should probably add a new item for openedx-events though, and mark it Provisional.

Standards-based Development > Provisional -> Accepted/Adopted: I am curious why this was put under Provisional. There are more than 3 example standards listed in the description that have already been implemented and widely adopted on the platform.

I was actually surprised when I drilled into the definition of this, because I had thought this was "Ed-Tech standards, like LTI, xAPI, and Common Cartridge", not "any kind of standard, including REST, OAuth, etc." I think that Standards in that sense is too vague to be its own item, especially when it conflates formally specified public interoperability specs like LTI and simpler library conventions like Flux Standard Actions.

@davidjoy: A few thoughts here:

  1. If we're tracking ed-tech standards, let's just kill the category and track those standards explicitly on the radar. LTI and xAPI are more important to us than something like OneRoster at the moment. Then we could park those specific standards in Adopted.
  2. If this item means, "we should build new things to talk to each other natively using accepted ed-tech standards whenever possible (rather than tacking them on afterwards)", that would deserve a separate callout/description. It would also be very provisional at the moment.

attrs > Adopted -> Accepted: Has there been much discussion about the learnings and tradeoffs with using attrs? My sense was that it's been starting to get traction, but not fully adopted yet with understanding on when to use and when not to.

I'm +1 on this.

Some minor comments on other pieces:

Frontend > Netlify (provisional): Are we just talking about using this as a static hosting service, or are there deeper ties to their API/build process? It does feel a little weird to have a specific service listed here (we don't have AWS for instance).

Technologies > django-storages (accepted): Haven't we effectively adopted this by now?

GraphQL (provisional): Is anyone planning to do even a proof-of-concept with this? I'm not opposed to leaving it in Provisional, but it feels kinda like a zombie.

davidjoy commented 2 years ago

Standards-based Development > Provisional -> Accepted/Adopted: I am curious why this was put under Provisional. There are more than 3 example standards listed in the description that have already been implemented and widely adopted on the platform.

I was actually surprised when I drilled into the definition of this, because I had thought this was "Ed-Tech standards, like LTI, xAPI, and Common Cartridge", not "any kind of standard, including REST, OAuth, etc." I think that Standards in that sense is too vague to be its own item, especially when it conflates formally specified public interoperability specs like LTI and simpler library conventions like Flux Standard Actions.

I don't disagree - admittedly that description was written quickly at the end when we were trying to get this out the door. I think you're providing a much more nuanced sense of what this means; we hit it with a big hammer and called it a day, at the time.

@davidjoy: A few thoughts here:

  1. If we're tracking ed-tech standards, let's just kill the category and track those standards explicitly on the radar. LTI and xAPI are more important to us than something like OneRoster at the moment. Then we could park those specific standards in Adopted.

Agree. 👍🏻 Calling out specific standards feels worthwhile - I wonder if it's worth leaving SBD in a more general sense as a "Technique" as a reminder that it's important to consider.

Some minor comments on other pieces:

Frontend > Netlify (provisional): Are we just talking about using this as a static hosting service, or are there deeper ties to their API/build process? It does feel a little weird to have a specific service listed here (we don't have AWS for instance).

I don't think Netlify belongs on the primary radar. It's something we use at edX primarily in Paragon builds for for deploy previews and to publish the documentation site.

GraphQL (provisional): Is anyone planning to do even a proof-of-concept with this? I'm not opposed to leaving it in Provisional, but it feels kinda like a zombie.

We use GraphQL in prospectus. Since that's a private repo, I think this doesn't belong on the primary radar.

Great improvements, @ormsbee.

sarina commented 2 years ago

Hi all! I'd love your review here: https://github.com/openedx/openedx-tech-radar/pull/7