Directory

⚓ T364652 Make component boundaries in MediaWiki core easier to discover
Page MenuHomePhabricator

Make component boundaries in MediaWiki core easier to discover
Open, MediumPublic

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:

Details

SubjectRepoBranchLines +/-
mediawiki/coremaster+4 -4
mediawiki/coremaster+3 -7
mediawiki/coremaster+47 -50
mediawiki/coremaster+137 -80
mediawiki/coremaster+2 -2
mediawiki/coremaster+16 -12
mediawiki/coremaster+23 -36
mediawiki/coremaster+36 -19
mediawiki/coremaster+7 -8
mediawiki/coremaster+2 -0
mediawiki/coremaster+15 -18
mediawiki/coremaster+15 -15
mediawiki/coremaster+130 -82
mediawiki/coremaster+61 -30
mediawiki/coremaster+48 -48
mediawiki/coremaster+1 -1
mediawiki/coremaster+151 -84
mediawiki/coremaster+42 -278
mediawiki/coremaster+287 -548
mediawiki/coremaster+10 -10
mediawiki/coremaster+7 -7
Show related patches Customize query in gerrit

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

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

[mediawiki/core@master] recentchanges: Improve docs, fix file headers, fix doc groups

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

Change #1064986 merged by jenkins-bot:

[mediawiki/core@master] recentchanges: Improve docs, fix file headers, fix doc groups

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

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

[mediawiki/core@master] docs: Set SORT_GROUP_NAMES=YES in Doxyfile

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

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

[mediawiki/core@master] recentchanges: Move rcfeed/ to includes/recentchanges/RCFeed/

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

Change #1065292 merged by jenkins-bot:

[mediawiki/core@master] docs: Set SORT_GROUP_NAMES=YES in Doxyfile

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

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

[mediawiki/core@master] objectcache: Improve WANCache docs

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

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

[mediawiki/core@master] objectcache: Improve overall BagOStuff class docs

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

Change #1068296 merged by jenkins-bot:

[mediawiki/core@master] recentchanges: Move rcfeed/ to includes/recentchanges/RCFeed/

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

Change #1068871 merged by jenkins-bot:

[mediawiki/core@master] objectcache: Improve WANCache docs

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

Change #1068959 merged by jenkins-bot:

[mediawiki/core@master] objectcache: Improve overall BagOStuff class docs

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

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

[mediawiki/core@master] language: MessageCacheUpdate

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

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

[mediawiki/core@master] language: Move Message classes under language/ directory

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

Change #1014148 merged by jenkins-bot:

[mediawiki/core@master] language: Move Message classes under language/ directory

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

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

[mediawiki/core@master] objectcache: Move RedisConnRef.php to /libs/objectcache/

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

Change #1083488 merged by jenkins-bot:

[mediawiki/core@master] objectcache: Move RedisConnRef.php to /libs/objectcache/

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

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

[mediawiki/core@master] ResourceLoader: Add ingroup to RL maint scripts

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

Change #1090522 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoader: Add ingroup to RL maint scripts

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

Krinkle triaged this task as Medium priority.

Change #1082530 merged by jenkins-bot:

[mediawiki/core@master] language: Move MessageCacheUpdate to language

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

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

[mediawiki/core@master] search: Move SearchUpdate.php to /includes/search directory

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

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

[mediawiki/core@master] lockmanager: Improve class docs and add example to wgLockManagers

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

Change #1091234 merged by jenkins-bot:

[mediawiki/core@master] search: Move SearchUpdate.php to /includes/search directory

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

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

[mediawiki/core@master] user: Write UserOptionsStore and User class docs

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

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

[mediawiki/core@master] user: Move UserOptionsUpdateJob to /includes/user/Options

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

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

[mediawiki/core@master] user: Tag UserArrayFromResult as `@internal` class

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

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

[mediawiki/core@master] objectcache: Move internal StorageAwareness to ObjectCache namespace

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

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

[mediawiki/core@master] objectcache: Tag SerializedValueContainer as `@internal`

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

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

[mediawiki/core@master] objectcache: Move RedisConn files out of utils/ to match class

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