Hi all!
In my opinion, MediaWiki’s PHPCS ruleset feels largely rooted in an older
version of PHP, where static type declarations (formerly known as “type
hints”) did not exist. As we move towards more modern code, I think some
rules should be relaxed, and others adjusted. More specifically, I’d like
to know if most people agree with the following propositions and conclusion:
Proposition 1: *Some code is sufficiently documented by names and types*,
and does not require additional documentation. Cases where additional
documentation is required do certainly exist, but they can only be
identified by human reviewers, not by automated tools.
You can see this in our existing code wherever a doc comment specifies only
a type (with @var, @param, or @return), but no additional text. For
example, in CreditsAction
<https://gerrit.wikimedia.org/g/mediawiki/core/+/de752f45af/includes/actions…>,
nobody needs to be told that the LinkRenderer will be used to render links,
or that the UserFactory creates User objects:
class CreditsAction extends FormlessAction {
/** @var LinkRenderer */
private $linkRenderer;
/** @var UserFactory */
private $userFactory;
Likewise, it’s not necessary to explain in great detail that the string
returned by LinksTable::getTableName()
<https://gerrit.wikimedia.org/g/mediawiki/core/+/de752f45af/includes/deferre…>
is the table name, that the $actor parameter of ActorCache::remove(
UserIdentity $actor )
<https://gerrit.wikimedia.org/g/mediawiki/core/+/de752f45af/includes/user/Ac…>
represents the actor to remove from the cache, or what the meaning of
the Message
$m and returned MessageValue are in Message\Converter::convertMessage()
<https://gerrit.wikimedia.org/g/mediawiki/core/+/de752f45af/includes/Message…>
:
/**
* Convert a Message to a MessageValue
* @param Message $m
* @return MessageValue
*/
public function convertMessage( Message $m ) {
(I want to clarify that in this last example I’m only talking about the
@param and @return tags that already don’t have any prose text. While the
method comment “Convert a Message to a MessageValue” might also be
considered redundant, I think this would be more contentious, and I’m not
advocating for removing that today.)
Proposition 2: *Adding types as static types is generally preferable.*
Unlike doc comments, static types are checked at runtime and thus
guaranteed to be correct (as long as the code runs at all); the small
runtime cost should be partially offset by performance improvements in
newer PHP versions, and otherwise considered to be worth it. New code
should generally include static types where possible, and existing code may
have static types added as part of other work on it. I believe this
describes our current development practice as MediaWiki developers.
Note that some older MediaWiki classes are considered unsuitable for static
types, and should only be used in comments; this is expected to help in a
future migration away from these classes (see T240307#6191788
<https://phabricator.wikimedia.org/T240307#6191788>).
Proposition 3: *Where types can be losslessly represented as PHP static
types, types in doc comments are unnecessary.* When doc comments are
considered necessary for actual documentation beyond types, then the type
is generally still included (and Phan will check that it matches the static
type), but when no further documentation is needed (see proposition 1
above), then the @var, @param, etc. doc comment can be omitted.
Note that depending on the PHP version, not all types can be losslessly
represented as PHP static types yet (e.g. union types and mixed both need
to wait for PHP 8.0, null and false for PHP 8.2); in such cases, doc
comments can remain necessary.
Conclusion: *We should update our PHPCS ruleset to require fewer doc
comments.* Exact rules are probably to be decided, depending on how much
work we’re willing to put into the sniff implementations (e.g. is it
feasible to require /** @param */ doc comments only if a parameter has no
static type?), but generally, I argue that we want code such as the
following to be allowed by our standard PHPCS ruleset:
class CreditsAction extends FormlessAction {
private LinkRenderer $linkRenderer;
private UserFactory $userFactory;
/** Convert a Message to a MessageValue */
public function convertMessage( Message $m ): MessageValue {
When doc comments are still necessary or at least beneficial because the
type alone isn’t enough information, it’s up to humans to decide this while
writing the code or point it out during code review.
What do people think about this? :)
PS: In PHP 8, we could abbreviate some of this code even more using
constructor property promotion:
class CreditsAction extends FormlessAction {
public function __construct(
Page $page,
IContextSource $context,
private LinkRenderer $linkRenderer,
private UserFactory $userFactory
) {
parent::__construct( $page, $context );
}
(Again, I’m not saying that all code should look like this – but I think we
have plenty of existing code that effectively carries no additional
information in its documentation, and which could be converted into this
form without losing anything.)
Cheers,
Lucas
--
Lucas Werkmeister (he/er)
Software Engineer
Wikimedia Deutschland e. V. | Tempelhofer Ufer 23-24 | 10963 Berlin
Phone: +49 (0)30-577 11 62-0
https://wikimedia.de
Imagine a world in which every single human being can freely share in the
sum of all knowledge. Help us to achieve our vision!
https://spenden.wikimedia.de
Wikimedia Deutschland - Gesellschaft zur Förderung Freien Wissens e. V.
Eingetragen im Vereinsregister des Amtsgerichts Berlin-Charlottenburg unter
der Nummer 23855 B. Als gemeinnützig anerkannt durch das Finanzamt für
Körperschaften I Berlin, Steuernummer 27/029/42207.
I'm getting "Can't connect to MySQL server on
'tools.db.svc.wikimedia.cloud" (115) from my various tools on toolforge -
as of about 10 minutes ago??
Arthur
Hello all!
The Search Platform Team usually holds an open meeting on the first
Wednesday of each month. Come talk to us about anything related to
Wikimedia search, Wikidata Query Service (WDQS), Wikimedia Commons Query
Service (WCQS), etc.!
Feel free to add your items to the Etherpad Agenda for the next meeting.
Details for our next meeting:
Date: Wednesday, December 7, 2022
Time: 16:00-17:00 UTC / 08:00 PDT / 11:00 EDT / 17:00 CET
Etherpad: https://etherpad.wikimedia.org/p/Search_Platform_Office_Hours
Google Meet link: https://meet.google.com/vgj-bbeb-uyi
Join by phone: https://tel.meet/vgj-bbeb-uyi?pin=8118110806927
Have fun and see you soon!
Guillaume
--
*Guillaume Lederrey* (he/him)
Engineering Manager
Wikimedia Foundation <https://wikimediafoundation.org/>
Hi everyone!
The fourth edition of the Coolest Tool Award
https://meta.wikimedia.org/wiki/Special:MyLanguage/Coolest_Tool_Award will
happen online on Friday 16 December 2022 at 17:00 UTC!
The event will be live streamed on Youtube in the MediaWiki (
https://www.youtube.com/user/watchmediawiki) channel.
See <https://zonestamp.toolforge.org/1642179615>
https://zonestamp.toolforge.org/1671210028 for your timezone.
The awarded tools will be showcased in a virtual event, with broadcasted
video and chat channels for socializing. We will send more details and
links soon.
Save the date, and join us celebrating the great work volunteer developers
do for the Wikimedia communities.
We hope to see you there!
Komla, for the Coolest Tool Academy 2022
--
Seyram Komla Sapaty
Developer Advocate
Wikimedia Cloud Services
This email is a summary of the Wikimedia production deployment of
1.40.0-wmf.12
- Conductor: Ahmon Dancy
- Backup Conductor: Brennen Bearnes
- Blocker Task: T320517 <https://phabricator.wikimedia.org/T320517>
- Status: Rolled out to all wikis
-
- 🔢 By the Numbers
Sparklines comparing with the last 5 trains.
- 565 Patches ▂▁▁█▇
- 0 Rollbacks ▄▁▁█▁
- 0 Days of delay ▁▁▁█▁
- 4 Blockers █▁▂▅▇
🥰 Trainbow Love 🥰Thanks to folks who reported or resolved blockers:
- Jon Robson
- Bartosz Dziewoński
- Subbu
- Matthias Mullie
- Taavi Väänänen
Hi all,
We have enabled the building of Mediawiki images for the Kubernetes cluster
during Scap sync operations, which includes Scap backports.
This means that during a backport window, your first backport will have to
wait for the images to be built and pushed remotely. From what we've seen
so far, you can currently expect this to add 3-6 minutes to the operation.
We are going to continue working on the feature and hopefully bring that
extra duration further down, but for the time being please be aware of the
extra time when performing a backport.
Best
--
Jaime Nuche
Software Engineer III
Wikimedia Foundation <https://wikimediafoundation.org/>
<https://wikimediafoundation.org/>
Hi all,
I have nominated Novem_Linguae for +2 rights in the PageTriage extension,
please feel free to comment at <https://phabricator.wikimedia.org/T324092>.
Kind regards,
*Sammy*
*(They/Them)*
User:TheresNoTime <https://meta.wikimedia.org/wiki/User:TheresNoTime>
*Unless otherwise stated, all statements from this account are made in a
volunteer capacity, and may not reflect the views of the Wikimedia
Foundation.*
*This message contains confidential information and is intended only for
the individual named. If you are not the named addressee, you should not
disseminate, distribute or copy this email.*