Talk:Best practices for extensions
This page used the Structured Discussions extension to give structured discussions. It has since been converted to wikitext, so the content and history here are only an approximation of what was actually displayed at the time these comments were made. |
Namespacing extensions
editRESOLVED | |
Clarified acceptable namespaces in the document. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
I see that the best practice for namespacing extensions would be MediaWiki\Extensions\ExtensionName
, but I'm aware that there's also extensions that follow this pattern:
MediaWiki\ExtensionName
(skips the Extensions
namespace prefix).
Examples include Linter (MediaWiki\Linter
) and GlobalUserPage (MediaWiki\GlobalUserPage
).
Which convention out of the two should people follow (or is it simply a recommendation/suggestion?) SamanthaNguyen (talk) 01:37, 18 December 2017 (UTC)
- I thought the convention was
MediaWiki\Extension\ExtensionName
(i.e. singular), and that's what I've been following (it seems least likely to result in collisions or confusion). There's some discussion in phab:T166010 of the options. Sam Wilson 02:04, 18 December 2017 (UTC) - Can’t believe I’ve actually been misreading it this whole time, so thanks. :P Thanks for the link too, that seems to clear it up a bit for me :) I’ll go with
MediaWiki\Extension\ExtensionName
too. SamanthaNguyen (talk) 03:56, 18 December 2017 (UTC) - The current consensus is that if the extension name is something unique and unlikely to be ever used in core (e.g. GlobalUserPage), then it's ok to be directly under MediaWiki. But if it's a generic-ish word like "Translate" then it should be disambiguated with
MediaWiki\Extension\Translate
. Legoktm (talk) 07:45, 18 December 2017 (UTC) - Hint taken :) Apropos, hopefully not too much off-topic, are there any recommendations for how to preserve some backwards compatibility (or not) when moving classes inside a namespace? I know that at least TranslateSVG and CentralNotice use some Translate classes. Nikerabbit (talk) 18:45, 18 December 2017 (UTC)
- For the record, I was just quoting Tim about "Translate" :)
- I'm not sure if we've documented this anywhere, but you should add a class_alias to the bottom of the file containing the new class (e.g. https://phabricator.wikimedia.org/source/mediawiki/browse/master/includes/libs/rdbms/database/Database.php;e1aabf2f24aef20adc72db8a750704cbb33236c6$3786) and then make sure the autoloader mapping still has the old class name pointing to the new file location.
- Also, Tim is currently working on a tool to automatically namespaceize core that should also be useable for extensions, so it might make sense to wait for that instead of doing it manually. Legoktm (talk) 23:54, 18 December 2017 (UTC)
Don't overuse private visibility in services
editRESOLVED | |
Removed the section. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
I have difficulty interpreting this point. Classes should not be injected with many services? Services should not have many properties? Their visibility should be something other than private? Tgr (WMF) (talk) 10:29, 24 May 2018 (UTC)
- The same idea is repeated further down in the document. I support the majority of this document, but this makes me sad. And I believe I can have an opinion on that because I still remember the time when I had the same idea. I was wrong: Services, classes, basically every code should only expose what it intentionally wants to expose. All kinds of interfaces (this includes class and function signatures) should be as narrow as possible, and only be opened up if an actual need to do so arises. Everything else should be private. Not protected, as protected is essentially "public to subclasses". Private by default.
- Can we change the point to "Use private by default" or something like "Function, class and interface signatures should be as narrow as possible"? Or, if we can not agree on this, at least remove the point for now? Thiemo Kreuz (WMDE) 16:59, 28 May 2018 (UTC)
- IIRC the idea was to make things protected rather than private so extensions have a chance to modify/extend things. I think it was a direct response to the "keep things as narrow as possible" concept because it makes external customization harder. Legoktm (talk) 16:00, 12 July 2018 (UTC)
- To me, this sounds like a misconception. When I introduce private properties or methods while working on the implementation details of a class, I don't do this to somehow make it easier for future subclasses to "modify/extend things". What "things" would that be, anyway? I typically have no idea if a class will ever have subclasses, and what needs these will have. I design an interface for what I know needs to be shared with the world. But the implementation I write is just that, implementation. Making things "protected by default" is literally exposing implementation details.
- Extension developers can easily submit a patch to make a method protected, if this is really what they need. Such a patch will typically (as I have seen it happen many times) start a brief discussion that will very quickly lead to a much better, much more sustainable architecture. Thiemo Kreuz (WMDE) 21:37, 12 July 2018 (UTC)
RESOLVED | |
Done. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
Right now this document uses language similar to but different from RFC 2119:
- REQUIRED – Meeting these criteria likely means your extension will work, but it may not be sustainable or maintainable.
- SHOULD – Meeting these criteria likely means your extension works well and will continue to work well in the future
- GOLD – Meeting these criteria means your extension is the ideal standard we all aspire to be, and should be used as an example to other developers
It might be better to directly use that RFC's language, e.g.:
- MUST – Meeting these criteria likely means your extension will work, but it may not be sustainable or maintainable.
- SHOULD – Meeting these criteria likely means your extension works well and will continue to work well in the future.
- RECOMMENDED – Meeting these criteria means your extension is the ideal standard we all aspire to be, and should be used as an example to other developers.
Thoughts? Jdforrester (WMF) (talk) 19:14, 11 July 2018 (UTC)
- +1 from me for using the RFC 2119 labels. BDavis (WMF) (talk) 19:37, 11 July 2018 (UTC)
- Ping people who have edited the page or this board for their views: @Legoktm, @Mainframe98, @Trizek (WMF), @Samwilson, @Quiddity (WMF), @SamanthaNguyen, @Thiemo Kreuz (WMDE), @Tgr (WMF), @Umherirrender, @Nikerabbit. Jdforrester (WMF) (talk) 22:04, 11 July 2018 (UTC)
- +1 from me. Quiddity (WMF) (talk) 22:17, 11 July 2018 (UTC)
- I support this change, makes sense to me. SamanthaNguyen (talk) 23:15, 11 July 2018 (UTC)
- Good idea. I been feeling like the 'GOLD' left a bit of room for "oh let's not bother", which RECOMMENDED doesn't so much. :) Sam Wilson 00:13, 12 July 2018 (UTC)
- Absolutely +1. —Mainframe98 talk 05:48, 12 July 2018 (UTC)
- RFC 2119 does not differentiate between SHOULD and RECOMMENDED; the levels are MUST, SHOULD and MAY (but the last one doesn't really match what GOLD means). Tgr (WMF) (talk) 07:14, 12 July 2018 (UTC)
- Yeah, I'm being a bit cheeky in seeing a distinction between SHOULD and RECOMMENDED which formally isn't there. Jdforrester (WMF) (talk) 18:28, 12 July 2018 (UTC)
- Go for Must, Should and Recommended while they are (I think) quite universally understood. Trizek_(WMF) (talk) 07:21, 12 July 2018 (UTC)
- Done in https://www.mediawiki.org/w/index.php?title=Best_practices_for_extensions&diff=2826422&oldid=2825537&diffmode=visual – hope this works for everyone. Jdforrester (WMF) (talk) 18:31, 12 July 2018 (UTC)
- thanks looks good! Sam Wilson 22:55, 12 July 2018 (UTC)
Version numbers in all files?
editRESOLVED | |
This item has been removed from the list of practices. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
Current wording: "Prominent version-number indication in the README and main PHP files (ideally all files)".
I take it this is for the version a file (or method, etc.) was introduced? Not that the current version number should be added to every file (apart from the readme and extension.json).
And also: do version numbers for extensions actually mean anything? What are they used for (other than for display in Special:Version)? Sam Wilson 22:58, 12 July 2018 (UTC)
- If a version number is not actively used as a tool for communication, it is indeed pretty worthless. More and more MediaWiki extensions just remove the version number because of this.
- This is different for smaller code libraries that – ideally – follow semantic versioning. These can also use
@since
tags – which is the only situation where I find version numbers helpful. - I would not copy-paste a version number around, even if an extension still uses one. What would be the benefit of that? The biggest problem with having the same version number in multiple "prominent" places is that they must all be updated. Every time. No, please don't do this. Have it in extension.json if it still means something for the main developers of an extension. Otherwise, just remove it. Thiemo Kreuz (WMDE) 18:40, 3 January 2019 (UTC)
Place for maintenance scripts
editRESOLVED | |
Maintenance scripts must be put into a maintenance/ directory. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
In the file layout, I think it should mention that maintenance scripts should be placed into directory named maintenance. Name scripts is also used a lot, but I'd rather not give options here like there is currently for src/includes modules/resources. Nikerabbit (talk) 12:46, 7 September 2018 (UTC)
- Done: https://www.mediawiki.org/w/index.php?diff=2874141&oldid=2866846&title=Best_practices_for_extensions&type=revision Legoktm (talk) 18:08, 8 September 2018 (UTC)
Injecting a logging channel for a class: Through constructor injection or setter injection?
editThe following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
This manual is useful, however, it does not specify whether it's recommended to inject the logging channel through a public setter or through the constructor for your object. Currently in MediaWiki core this seems to be inconsistent, which makes it confusing. Here is to note how all core services defined here ( https://github.com/wikimedia/mediawiki/blob/master/includes/ServiceWiring.php ) currently do it:
\MimeAnalyzer
: Constructor\MediaWiki\Storage\NameTableStoreFactory
: Constructor\OldRevisionImporter
: Constructor\MediaWiki\Preferences\PreferencesFactory
: Setter\MediaWiki\Revision\RevisionRenderer
: Setter\MediaWiki\Revision\RevisionStoreFactory
: Constructor\MediaWiki\Shell\CommandFactory
: Setter\UploadRevisionImporter
: Constructor\ImportableOldRevisionImporter
: Constructor
From my knowledge, a service **must** be immutable. By providing the logger to the object through a setter means it's can be changed even after instantiation, which goes against practice of dependency injection. Plus, with the fact that it can still be changed, it means it's harder to debug which seems to be counter-intuitive. in the document, it mentions SHOULD: Use dependency injection
under the Architecture section. Personally, I prefer to use constructor injection in general.
I believe we should have a standard on this, and this document should be suggesting which way. What is the benefit of having a class implement the \Psr\Log\LoggingAwareInterface
and use the \Psr\Log\LoggerAwareTrait
to satisfy the interface instead? Shouldn't it always be done through the constructor? I feel like this is a discussion worth having; please let me know if I've misunderstood. thank you. SamanthaNguyen (talk) 21:23, 24 December 2018 (UTC)
- The one problem with constructor injection is – even if clearly to be preferred – that one might end with a constructor with a dozen or more parameters. The constructor becomes harder to use. Parameters might get confused (if they don't have strict type hints, or adjacent types are the same). Setting up additional test cases is a pain because of all the services that need to be mocked and injected, but don't matter for the particular test case. A situation like this hints at a higher-level design issue: A service that needs so much stuff to be injected probably does way to many unrelated things, and should be broken up.
- With this out of the way my personal rule of thumb is a pretty strict "dependency injection at construction time". But there is one exception: loggers. See, a logger does not really change the behavior of a service. It is merely a side-effect if what the service does additionally creates log entries – or not. Even if the logger is changed mid-way, nothing happens.
- I still prefer constructor injection, even for loggers. But if the constructor is already quite big (let's say 4 or more arguments), or I want to temporarily add logging to an existing class, I avoid making the constructor more complicated. A setLogger method that defaults to a NullLogger if not called (super useful in test situations) is easy to add – and to remove. Thiemo Kreuz (WMDE) 19:13, 3 January 2019 (UTC)
- Thiemo: Thank you for the elaborate answer! I believe this answers my question. SamanthaNguyen (talk) 03:43, 4 January 2019 (UTC)
Rating for "uninstallation maintenance scripts"
editRESOLVED | |
Done. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
There's no rating for this item under the Database section. Is it something that must be done, should be, or recommended? SamanthaNguyen (talk) 18:58, 25 December 2018 (UTC)
- I didn't see this at the time, but I added "should" in front of it last year in this edit as part of a general formatting copy-fix. I didn't intentionally make a choice there. I've changed it to "optional" now because 1) This is something virtually no extension does today so it seems only right to begin there given that these are meant to reflect current practice, and 2) There is a related item about uninstall instructions under "Documentation" which is also optional already. Krinkle (talk) 02:47, 27 January 2022 (UTC)
Don't: subclass anything not intended to be subclassed by extensions
editRESOLVED | |
Done, per Stable interface policy. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
I propose to add the following to the "Don't" section:
- MUST NOT: subclass (extend) any class defined by MediaWiki core, unless that class is explicitly documented to allow subclassing by extensions.
I'm tempted to even extend this to implementing interfaces. Not all interface declarations are intended as extension points, and not all of them should be considered stable. If we go that far, we should also add:[...] the stable part of the PHP API is comprised of all code that is explicitly marked public, and has been included in at least one stable release. In addition, some classes expect to be subclassed in extensions; in those cases protected functions also are included in the API. These classes should have a note in their documentation comment that they expect subclassing. If no note is present, it SHOULD be assumed that the class is not expected to be subclassed.
- MUST NOT: implement any interface declared by MediaWiki core, unless that interface is explicitly documented to allow implementation by extensions. DKinzler (WMF) (talk) 16:06, 27 March 2019 (UTC)
- +1 to both suggestions from my side.
- I, personally, find it a really helpful rule-of-thumb to consider all classes to be
final
, except otherwise stated. The effect of this restriction is pretty much the same as you suggest: one can not extend anything, except it is allowed. The only reason we are not literally marking all our code asfinal
is that it would be cumbersome and error-prone. But we should still threat it like it is. Thiemo Kreuz (WMDE) 16:31, 27 March 2019 (UTC) - This has been superseded by the Stable interface policy. Krinkle (talk) 02:49, 27 January 2022 (UTC)
Don't: directly construct service objects
editRESOLVED | |
See Stable interface policy. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
In a way similar to protected methods, constructor signatures of service-like objects are generally not stable. They are intended for use by wiring code, and can be expected to change without notice. Extensions (and all application logic) should ideally only ever directly construct plain value objects. However, we still have quite a number of things in core that are neither service objects nor value objects. So I propose to add the following to the "Don't" section:
- MUST NOT: directly instantiate a service class defined by core, instead of obtaining an instance via a service container.
- SHOULD NOT: directly instantiate anything that is not a plain value object. DKinzler (WMF) (talk) 16:10, 27 March 2019 (UTC)
- Sounds good. Jdforrester (WMF) (talk) 19:06, 27 March 2019 (UTC)
- Sometimes you need a service with non-default configuration, most commonly when doing cross-wiki things. (Random recent example: [1]) Tgr (WMF) (talk) 22:19, 11 April 2019 (UTC)
- As long as that is in core, it could be acceptable. Extensions should never do that.
- The Right Way (tm) is to introduce a factory service that you can ask for a service instance for the target wiki.
- Eventually (tm) we'd want a full MediaWikiServices instance for the target wiki. That will be possible as soon as no service uses global state. DKinzler (WMF) (talk) 09:24, 12 April 2019 (UTC)
- We won't realistically introduce factories for all the services which depend on the wiki (which is basically all the services), MediaWikiServices doesn't support using foreign wiki configuration yet (even if it will, that will only cover wikis in the same farm, which is not the only use case), and expecting extension authors to make core changes instead of doing simple workarounds in their extensions is not realistic or reasonable IMO (especially as long as we don't get our code review problems under control). Tgr (WMF) (talk) 15:29, 12 April 2019 (UTC)
- > MediaWikiServices doesn't support using foreign wiki configuration yet
- Actually, it does. But not all services are completely isolated from global state.
- > expecting extension authors to make core changes instead of doing simple workarounds in their extensions is not realistic or reasonable IMO
- These workarounds are what is preventing core changes, because we break them when we change core. I would expect extension authors to request or propose core changes when they need them, yes. DKinzler (WMF) (talk) 12:08, 16 April 2019 (UTC)
- Reconsidering what I just said: extensions may have a need to call constructors of services if they extend the service class, or if the replace the service instance. That is, they may need to call service constructors in wiring code.
- This is fine as long as the service is declared to be extensible - which implies that the constructor signature is treated as public and stable. Constructor signatures of other services should not be considered stable.
- The rationale for this restriction is to support better dependency injection: with DI, there is often a need to change constructor signatures. Which is generally no problem since DI says that the constructor is only called in the wiring code, so there is only one place that needs to be changed to accommodate the new signature. Making such changes backwards compatible is a burden that should be avoided when possible - that is, we should only pay that cost in cases where the service is intended as an extension interface. DKinzler (WMF) (talk) 13:03, 16 April 2019 (UTC)
- > Actually, it does.
- Actually, it doesn't :) There is no way for me to obtain a DI container for a foreign wiki, short of constructing that container myself using a fake configuration object that I also construct myself, which is a lot more nasty than constructing a specific service manually using a specific wiki ID and language object.
- > I would expect extension authors to request or propose core changes when they need them, yes.
- That's a reasonable expectations for WMF staff maintaining extensions used in Wikimedia production. It's a completely unreasonable expectation for people running their own wikis, possibly in their free time, their bug reports get mostly ignored and their patches spend months if not forever in Gerrit without anyone ever looking at them. The MediaWiki community just does not provide the level of volunteer support currently where we could in good conscience ask people to have their problems fixed in core.
- More specifically to the foreign wiki issue, what would the core change be? Do we really want to create a factory class for everything that takes a wiki ID, when those classes won't be necessary once MediaWikiServices supports foreign wikis? Tgr (WMF) (talk) 17:13, 16 April 2019 (UTC)
- > There is no way for me to obtain a DI container for a foreign wiki, short of constructing that container myself using a fake configuration object
- Right - the problem is that there is no clean way to get the config settings for another wiki. For this to be complete, it would have to include the enabled extensions as well, so we'll need injecteble hook runners.
- > It's a completely unreasonable expectation for people running their own wikis, possibly in their free time, their bug reports get mostly ignored and their patches spend months if not forever in Gerrit without anyone ever looking at them.
- This policy doesn't apply to people writing their own extension for their own use. But yes, their extension might break without warning.
- > More specifically to the foreign wiki issue, what would the core change be? Do we really want to create a factory class for everything that takes a wiki ID, when those classes won't be necessary once MediaWikiServices supports foreign wikis?
- I'm not particularly keen on doing that, but the code is trivial enough. We could have a helper class or a trait for the boiler plate, even. DKinzler (WMF) (talk) 17:25, 16 April 2019 (UTC)
- This page is definitely meant for people writing their own extensions, so that they end up with maintainable code that others can reuse. Although I guess it would make sense to treat it as a policy (which it currently isn't) for Wikimedia-deployed extensions and merely as guidance for others. Tgr (WMF) (talk) 19:12, 16 April 2019 (UTC)
- You are right, of course - it is meant for them, but thy are not bound by it. Sorry for being imprecise. DKinzler (WMF) (talk) 19:22, 16 April 2019 (UTC)
- The point is, don't add MUST NOTs that are not reasonable expectations for most of the intended audience (even if they are not bound by it). Tgr (WMF) (talk) 20:26, 16 April 2019 (UTC)
- I think they are reasonable expectations. The only way to get out of the we-can't-change-core-without-breaking-extensions dead end is to limit what extensions can do (or what they can expect to continue working with the next release).
- But you are right about my original proposal being too restrictive. It should be more along the lines of:
- MUST NOT: directly instantiate a service class defined by core, instead of obtaining an instance via a service container, except when wrapping, replacing or defining a service that is documented to be an extension point.
- Another way of phrasing this would be
- MUST NOT rely on the stability of a constructor signature of a class that is not defined to be a pure value object, or documented to be an extension point suitable for subclassing or direct construction by extensions.
- The second option is not less restrictive, but perhaps more to the point.
DKinzler (WMF) (talk) 09:55, 18 April 2019 (UTC)- I believe this is now covered by Stable interface policy. Krinkle (talk) 02:50, 27 January 2022 (UTC)
Deprecate /includes
editRESOLVED | |
Done, we now recommend PSR4 and a "/src" directory. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
See comment at Manual talk:Coding conventions/PHP#Directory Structure. I agree; we should follow standard PHP naming conventions and use /src for at least PSR-4 extensions. Tgr (WMF) (talk) 22:22, 11 April 2019 (UTC)
- +1 for src/ in new projects. includes/ is a very PHP3/4 convention. BDavis (WMF) (talk) 00:10, 12 April 2019 (UTC)
Avoid global state
editRESOLVED | |
Added. |
The following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
One thing that should be avoided even more than global state (e.g. global variables/public static properties in classes) is local static vars and similar instances of hidden local state, especially when used for caching. It looks like a very good idea when writing it, and is absolute nightmare when debugging/testing, because it creates hidden un-clearable state which could be in any random configuration when you get to it. Smalyshev (WMF) (talk) 01:09, 12 April 2019 (UTC)
- There is an other document in the works at User:DKinzler (WMF)/Software Design Practices that covers exactly this concern very well, I find. Thiemo Kreuz (WMDE) 16:01, 8 May 2019 (UTC)
- I've made the point about global state explicit in this edit. Krinkle (talk) 17:57, 2 March 2022 (UTC)
Location for source JS files and JS build artifacts
editThe Best practices document states that the `/src` folder is a place only for PHP files. The problem I noticed is that some extensions put more detail to the Javascript side, for example MobileFrontend or PagePreviews use lots of JS code. Further more, those extensions use Webpack - building step that transforms src js files into compiled/minified code.
The webpack best practices state that the source code should go to `/src` folder, and the compilation artifacts to `/dist`, but that doesn't match the MediaWiki practices which states that JS code should live in the 'resources'.
Additionally, having a possibility to keep "new" js files in different place than `/resources` folder gives us an easy way to see which parts of the JS code are already ported, and which waits for the refactor.
Currently, Popups and MobileFrontend extensions have the JS code located in the `/src` folder, and the compilation artifacts stored in the `/resources/dist` folder. PMiazga (WMF) (talk) 19:46, 3 June 2019 (UTC)
- Keeping resources/dist, moving PHP to src/php, and moving JavaScript to src/js works for me. Alternatively, src/server and src/client also work. Niedzielski (talk) 19:54, 3 June 2019 (UTC)
- +1 to both @PMiazga (WMF) and @Niedzielski. Outside of mediawiki, `/src` is a very common convention for JS. If PHP follows that convention as well, @Niedzielski suggestion might be best. NRay (WMF) (talk) 20:18, 3 June 2019 (UTC)
- Thanks @PMiazga (WMF) for starting this thread. Conventionally, I see a lot of code bases using `src/` for putting human readable code but over the years, MediaWiki and extensions have adopted and used `includes/` for this purpose. I'm not sure exactly why `includes/` and the reasoning behind it but it would be good to know.
- That being said, there are 3 questions in my mind right now;
- 1. Will making changes to the directory structure a little bit (moving from `includes/` to `src/`) reduce cognitive load for current developers and its community at large? It's kinda hard for 10 years MW developers to make this kind of switch?
- 2. Will this migration/change impact wikis (Wikimedia or others) configuration wise? How good or bad will the impact be?
- 3. Will the new directory structure be easier for new developers to adopt and navigate the codebase?
- While I've asked those questions, I'll lean towards adopting the `src/` directory structure for placing our source code files (PHP + JS) as it's pretty much used almost everywhere.
- In addition, @Niedzielski also raised a good point about `src/php` and `src/js`. Could this be an RfC for more people to give ideas and discuss about it? X-Savitar (talk) 09:09, 4 June 2019 (UTC)
Location for Mustache files
editAs a skin developer, I hope to know if there is a suitable place for Mustache files.
In my brief investigation, the places for mustaches are:
/templates
in Extension:WikibaseMediaInfo, extension:Collection, Extension:ChessBrowser, Extension:Quiz, Extension:MediaSearch, Skin:Cologne Blue, Skin:Modern, Extension:CentralNotice and so on./includes/templates
in Skin:Vector and Extension:MobileFrontend/includes/specials
in Extension:MobileFrontend/modules/templates
in Extension:ArticleFeedbackv5 and Extension:PerformanceInspector/modules/tools
in Extension:ContentTranslation/skin/templates
in Extension:ContentTranslation/includes/Skins
in Skin:Minerva Neue and Extension:CreateAPage/resources/templates
in Skin:Mirage/components
in Skin:WikimediaApiPortal and Skin:Minerva Neue/handlebars
in Extension:StructuredDiscussions (using handlebars)
Should /template
be the best location?
cc: I found @Jdlrobson had chosen /templates
for Skin:Example. (3ef4c54
) Could I request for your comment? Lens0021 (talk) 11:21, 22 March 2021 (UTC)
/includes
is for PHP-specific files and/modules
is for JS and other frontend-specific files. If you only use the templates in PHP or in JS, I guess those choices make sense. Otherwise I'd go with/templates
. Tgr (WMF) (talk) 15:40, 22 March 2021 (UTC)- For Skin:Mirage, I chose
/resources/templates
because I wanted to be able to re-use the templates in JS front-end. I ended up not needing that (at least not yet), but left it there for the future. —Mainframe98 talk 15:47, 22 March 2021 (UTC)
What is this page's scope?
editFrom the name and (extremely minimal) lede, it sounds like this page is making recommendations for all extensions. However, from the very first recommendation, it's clear that the intended scope is only extensions that are meant to be used generally, and especially those aiming for WMF deployment. This disconnect could lead readers to believe that an extension being developed exclusively for use on e.g. a single third-party wiki is either explicitly discouraged, or has to jump through unexpected hoops that implicitly discourage development of such extensions. I'm sure that such extensions shouldn't be encouraged, but I think it's a mistake to actually discourage their creation/use, explicitly or otherwise.
tl;dr This page's lede should be expanded to clarify what sorts of extensions it's intended for: all extensions, or only those intended for use outside of their originating communities/wikis. (Possibly individual recommendations should receive similar clarifications.) 「ディノ奴千?!」☎ Dinoguy1000 07:02, 27 January 2022 (UTC)
- The document is intended for all extensions, but I think it is worth recognizing that while something is universally a best practice, it may not be applicable for specific extensions. For example, disabling the parser cache is not a good thing, but some extensions do it, and that's okay for how they're used.
- The first recommendation is "Use the standard issue tracker". I don't understand why you think that's specific to WMF deployment. In general, having MediaWiki extensions track bugs in Phabricator and use Gerrit (eventually GitLab) for code review should be considered a best practice, given all the benefits from doing so. Legoktm (talk) 07:24, 27 January 2022 (UTC)
- Not all MediaWiki wikis are owned/operated by/as public, FLOSS, and/or not-for-profit. Accordingly, all MUSTs are at best OPTIONALs for basically obvious reasons n.b. it's not reasonable to expect a private company with likely different expectations of licensing (the polar opposite of those three values, so the other end of the spectrum) to comply with "publish on WMF Gerrit" and "use WMF Phabricator" and "use PSR-4" and "localize" and etc. For example, even two other organizations that are reasonably 'open' and in a topical sphere to WMF (Miraheze, Fandom) don't do every one of those MUST things.
- So, I agree that the scope should be clear, as should be the expectations associated with not developing with the requirements, objectives, and best practice as documented here in mind. Izno (talk) 08:47, 27 January 2022 (UTC)
- This document isn't targeted towards private companies who want to make their own extensions. I think it is reasonable to have a baseline for *best practices* to use localization (one of MediaWiki's Principles), PSR-4 (a PHP best practice), and use Gerrit/Phabricator to be along with the rest of the MediaWiki ecosystem. Legoktm (talk) 07:53, 4 February 2022 (UTC)
- I don't think it's targeted at them, but it can trivially be read as if it were. Separately, because it leans on the RFC text as it does (I am increasingly not a fan in the way it changes the meaning of those words from the RFC), someone who does look at any given line with or without context will surely not see the forest from the trees when "MUST" is the word used.
- This document should cater to professional engineers because the advice it gives is valuable for anyone developing an extension, not just the open source or trivially-reached MediaWiki ecosystems. That's besides the already-provided organizations which are trivially-reached MediaWiki ecosystems and which themselves have an interest in observing these practices, yet clearly will not and do not have an interest in e.g. using WMF Gerrit.
- GOLD/SILVER/BRONZE, or BEST/BETTER/CONSIDER/RECOMMENDED, or similar other words would be better on that point. Remember, these are best practices. Izno (talk) 17:39, 4 February 2022 (UTC)
- +1 to MUST/SHOULD/OPTIONAL vocabulary not really being useful for general guidelines. You MUST use Phabricator and Gerrit and MUST put your classes in an
src
directory... or else what? Plenty of MediaWiki extension do not follow these rules; plenty of Wikimedia extensions do not follow many of the MUST rules, even. And I struggle to see what moral or legal authority anyone would have to make such declarations for plugins for a FLOSS software. - There are a few security-related points that we'd probably enforce by dissociating problematic extensions from the project or warning users against them, but everything else is a recommendation. Maybe rename it as good / great / best or something like that? (The API best practices guideline uses gold/platinum, it doesn't have a third level though.) Or preserve MUST for the few things where, if we had an extension store, we would kick an extension out of it if it violated that. Tgr (WMF) (talk) 09:34, 27 January 2022 (UTC)
… or else you are not compliant with this document, and we would not call your extension "best practice", as judged by the standards of the MediaWiki ecosystem.+1 to MUST/SHOULD/OPTIONAL vocabulary not really being useful for general guidelines. You MUST use Phabricator and Gerrit and MUST put your classes in an
src
directory... or else what?
Indeed. Why do you think this page is claiming it has such authority? Jdforrester (WMF) (talk) 22:58, 27 January 2022 (UTC)Plenty of MediaWiki extension do not follow these rules; plenty of Wikimedia extensions do not follow many of the MUST rules, even. And I struggle to see what moral or legal authority anyone would have to make such declarations for plugins for a FLOSS software.
- I specifically didn't speak to the RFC language since Talk:Best practices for extensions#h-Align_language_to_RFC_2119?-2018-07-11T19:14:00.000Z (second discussion from the bottom) is pertinent on that point. Izno (talk) 18:08, 27 January 2022 (UTC)
OOUI
editShould there be something here about using OOUI when possible to create interface elements, instead of using the Html class or (*gasp*) hardcoded HTML?
By the way, the document does seem to say that the Html class should be used instead of hardcoded HTML, but maybe that part could be clearer. Yaron Koren (talk) 14:29, 1 February 2022 (UTC)
- Given that OOUI is now deprecated, I think we should avoid giving that guidance right now. But sadly the alternative (Codex) isn't ready and a server-side-rendering system certainly isn't. Ah well. Jdforrester (WMF) (talk) 20:18, 1 February 2022 (UTC)
- I didn't know OOUI was actually deprecated. Not to get into in a side issue, but is it really officially deprecated, or is it just sort of implicit that, given that a competing library is being developed, OOUI is not going to get much attention any more?
- In any case, this doesn't seem like a satisfactory answer. If an extension's interface is going to adhere to the Wikimedia Design Style Guide (which is also not mentioned in this document, by the way), OOUI is still the easiest way to achieve that, as far as I know. Let me ask this: if a new extension were created today for use on Wikipedia or other Wikimedia sites, and it contained form elements, how would its UI be coded so that it matched the style guide? Or would it simply have the default HTML gray buttons? (Doubtful.) Yaron Koren (talk) 02:26, 2 February 2022 (UTC)
- Re: OOUI's deprecation, see this wikitech-l thread.
- My understanding is that practically a new MediaWiki extension would most likely use OOUI because it has the most comprehensive widget library and is well integrated into everything...Codex isn't there yet AIUI. I think leaving that as a TODO or even a note on the page saying it's in flux would be okay. Legoktm (talk) 07:58, 4 February 2022 (UTC)
- Codex is currently technically vapourware (there's not even an alpha release), but I have faith that it'll be usable for JS-only interfaces in the next few months (disclaimer: my team are one of those trialling it).
- Codex-with-SSR (which could be used for non-JS users and to avoid FOUC) is ultra-vapourware with no plan let alone code written. It'll be a while. Jdforrester (WMF) (talk) 15:23, 4 February 2022 (UTC)
"Regularly submit and receive translations from translatewiki.net"
editThe following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
It's a good idea for extensions to regularly receive translations from translatewiki.net (not that we really have a choice in the matter), but what does it mean to regularly submit them? Yaron Koren (talk) 14:36, 1 February 2022 (UTC)
- I've added:
I copied and summarised this from the translatewiki.net page, which was previously linked from the same bullet point. Krinkle (talk) 17:36, 1 February 2022 (UTC)For extensions hosted in Wikimedia Gerrit, translatewiki.net staff will generally proactively do this for you. They start an automatic process which subscribes to new messages from your extension, and also automatically exports and merges updated translations back into your repository once a day. If this hasn't happened within a week, contact TWN staff.
- Thanks, that looks good to me. Yaron Koren (talk) 16:55, 2 February 2022 (UTC)
Confusing A11y guideline about ids
editI'm confused by this item in the section about Accessibility:
SHOULD: HTML element's IDs should only be rarely, for things to which you can navigate. Everything else should be a class – even if you expect to use it only once, use a class name instead.
Why would that be an accessibility consideration? Most accessibility uses for ids aren't related to navigation and can't be replaced by classes (for example, the target of an aria-described-by=""
and such).
I can see that ids should not be used for styling, but that is not an accessibility concern. And I can see that ids must be unique, but that is a MUST and also affects much more than only accessibility.
Am I missing something here? Michael Große (WMDE) (talk) 14:41, 28 February 2022 (UTC)
- Correct. however those should generally be auto-generated ids (again to guarantee uniqueness), but yes, this is more of a code convention than an accessibility concern. —TheDJ (Not WMF) (talk • contribs) 19:23, 28 February 2022 (UTC)
- I'd generally like to remove all content under Best practices for extensions#Accessibility and instead link to Accessibility guide for developers to avoid quadruplicating more and more guidelines. AKlapper (WMF) (talk) 08:51, 1 March 2022 (UTC)
- @AKlapper (WMF) That sounds like a sensible approach to me. Michael Große (WMDE) (talk) 10:41, 2 March 2022 (UTC)
- Done. AKlapper (WMF) (talk) 13:01, 2 March 2022 (UTC)
- But none of that is written in terms of should/must, so now we're adding a fudge factor back into the process, whereas this entire document is meant to be a "one-stop-shop" where if you pass all the items you can be pretty sure you could as 'best practice. Jdforrester (WMF) (talk) 17:25, 2 March 2022 (UTC)
- @Jdforrester (WMF) Please feel free to edit it and define MUSTs and SHOULDs. Thanks! AKlapper (WMF) (talk) 17:38, 2 March 2022 (UTC)
- I'd rather you did that, as the editor of this area here, Andre. Jdforrester (WMF) (talk) 17:58, 2 March 2022 (UTC)
- OK, it's been a month, we can just revert your removal Andre if you don't have time to do the work? Jdforrester (WMF) (talk) 14:58, 28 March 2022 (UTC)
- Surely we can duplicate content (and likely make things go out of sync), however I don't think it's a good idea when there are already guidelines for an area to add yet another place with guidelines for that area. For the time being, feel free to go ahead. AKlapper (WMF) (talk) 15:57, 28 March 2022 (UTC)
bad links for doclink/html and doclink/Sanitizer
editThe following discussion is closed. Please do not modify it. Subsequent comments should be made on the appropriate discussion page. No further edits should be made to this discussion.
the two targets of doclinks cannot be reached:
<!--T:161--> Use MediaWiki's validation/sanitization methods ''e.g.'' those in the {{class doclink|Html}} and {{class doclink|Sanitizer}} classes.
please correct or realign, thanks --Christian 🇫🇷 FR (talk) 10:33, 16 April 2024 (UTC)