Page MenuHomePhabricator

Make component boundaries in MediaWiki core easier to discover
Open, Needs TriagePublic

Assigned To
None
Authored By
Krinkle
May 10 2024, 7:59 PM
Referenced Files
None
Tokens
"Yellow Medal" token, awarded by Mooeypoo."Stroopwafel" token, awarded by ssastry."Orange Medal" token, awarded by Ladsgroup."Like" token, awarded by Izno.

Description

There are a handful of areas in MediaWiki core that we've historically treated as a vertical. This stems from a time where WMF was smaller, and MediaWiki was largely maintained as a single unit. In that time, the notion of a dedicated teams owning a only specific feature within core was the exception rather than the rule, so we've always handled it without much structure, folding it into the larger hole.

Motivation

Especially for Gerrit queries, this proposal should not only reduce duplicate effort but also makes commits affecting a component more likely to be discoverable in the first place, thus reduce code review latency and increase inter-team awareness of relevant changes, as people are currently unlikely to bother listing out each of these files, not to mention when files are moved or renamed, or when new files are added.

Typically, it goes as follows:

  • The main business logic of a feature goes in /includes/something. Nowadays this includes e.g. relevant interfaces, value objects, and service classes.
  • If the feature uses the JobQueue, we defined a SomethingJob in /includes/jobqueue/jobs/, which is generally small and simply calls the Something service class.
  • If the feature uses DeferredUpdates, we place its classes in /includes/defered/, which are generally small and call the Something service class.
  • If the feature has a user-facing API, we define ApiSomething (ApiBase subclass) to /includes/api/, which likewise calls the Something service.
  • If the feature has a user-facing GUI, we define SpecialSomething in includes/specials/.
  • If the feature stores its own data, we define tables within /maintenance/tables.json.

There may also be sysadmin-facing maintenance script in /maintenance/, and/or frontend resources under /resources/src/mediawiki.something/.

Impact

For most core components, you typically have only 2 or 3 of the above. Yet, this is enough to end up duplicating a fair bit of information in many different places, which are typically not in sync, or simply missing all but the first entry.

Scope

As part of this specific task, I propose we handle these two areas only. This keeps the task focussed and scoped to consensus on the direction rather than specific details, which for other areas can be handled in separate tasks, after the direction is agreed upon. It may also be spread out over a larger period of time, with separate goals for separate areas. In particular, I want to avoid a rushed conclusion where many more times amounts of hidden work are required afterwards where the "real" decisions are made.

  1. JobQueue jobs from /includes/jobqueue/jobs/
  2. Deferred updates from /includes/deferred/

We can start with deferred updates. For deferred updates, there are a few internal classes (that contain no business logic!) that exist to be extended by specific deferred updates, such as AtomicSectionUpdate. These are part of the DeferredUpdates system and the API this system provides to extensions. These will not move. What we move are the edges that implement specific features, e.g. SearchUpdate would move to /includes/search/, and MessageCacheUpdate to /includes/language/.

We can then repeat this for jobs where there is clearly 1 component a class belongs to, and where that component has its own well-named directory. For the handful that are left, we can investigate what is needed for there to be a clear component (e.g. a minor edit to a description or non-PSR directory name may be required to recognise if a directory's scope has changed over the past decade). Note that NullJob and DuplicateJob are internal classes leverared by the JobQueue system itself, and would not move.

Misc notes

In-take for tasks in a sustainable way, encourage components and teams, not broad categories. This motivated these changes:

Event Timeline

Change #1030305 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] Move various job classes to relevant component directories

https://gerrit.wikimedia.org/r/1030305

Change #1030326 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] watchlist: Move un-namespaced watcheditem classes to /includes/watchlist/

https://gerrit.wikimedia.org/r/1030326

virtualenv --system-site-packages reviewer-bot && cd reviewer-bot
source bin/activate
git clone http://github.com/valhallasw/gerrit-reviewer-bot src
cd src
pip install -r requirements
git clone https://gerrit.wikimedia.org/r/pywikibot/core pywikibot
cd pywikibot && git checkout 2.0

Change #1030305 merged by jenkins-bot:

[mediawiki/core@master] Move various job classes to relevant component directories

https://gerrit.wikimedia.org/r/1030305

Change #1030326 merged by jenkins-bot:

[mediawiki/core@master] watchlist: Move un-namespaced watcheditem classes to /includes/watchlist/

https://gerrit.wikimedia.org/r/1030326

Change #1031057 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] specials: Improve docs and `@ingroup` tags, fix file headers

https://gerrit.wikimedia.org/r/1031057

Change #1031057 merged by jenkins-bot:

[mediawiki/core@master] specials: Improve docs and `@ingroup` tags, fix file headers

https://gerrit.wikimedia.org/r/1031057

Krinkle edited subscribers, added: ssastry; removed: Osps7.EditedMay 21 2024, 9:46 PM

Was hoping to discuss this in MwEng leadership triage meeting, but it hasn't happened for several weeks so \cc-ing @ssastry @Bmueller directly, in relation to the "WE 3.3 Code ownership" KR for FY2024-2025. Looking forward to discuss it next time we chat.

I think the description could be edited for clarity -- we can discuss when we chat next. But, your gerrit patches helped clarify your proposal -- and I like it overall. Component boundaries in MW was something that came up in discussions in the code ownership working group and moving in this direction will make it possible to better isolate components for ownership (with all the other attendant benefits in gerrit, phab, wiki, etc. around simplifying descriptions).

Change #1043884 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/core@master] language: Widen `@covers` tags in phpunit tests

https://gerrit.wikimedia.org/r/1043884

Change #1043884 merged by jenkins-bot:

[mediawiki/core@master] language: Widen `@covers` tags in phpunit tests

https://gerrit.wikimedia.org/r/1043884