Page MenuHomePhabricator

Evaluate the future of wikibase-codesniffer
Closed, ResolvedPublic

Description

wikibase-codesniffer uses mediawiki-codensiffer
mediawiki-codensiffer has advanced quite some since wikibase-codesniffer was created.

Perhaps we can stop maintaining a separate thing and instead just use mediawiki-codesniffer directly.

Event Timeline

An example of how the extra level of indirection creates overhead will be seen in DataModel soon, where:

  • T253633 will be fixed in mediawiki-codesniffer
  • A new version of mediawiki-codesniffer is then needed
  • wikibase-codesniffer then needs to be updated to use the new mediawiki-codesniffer
  • wikibase-codesnifer then needs to be released
  • Then we could do T253636 using the new version of mediawiki-codesniffer in DataModel

There are possibly other ways to solve this than just removing wikibase-codesniffer entirely, but I believe the evaluation should determine if we actually need it / what we currently gain from it

I honestly think we should just drop wikibase codesniffer. We have been maintaining too much codebase.

Note that phpcs ruleset is controllable per repository. The same way that we disable some rules in repositories not yet ready to follow a newer convention, it is also possible to enable additional rules on a per-repo bases.

So these rulesets could easily be put into the phpcs files of the repositories in question directly. If there are also PHP classes involved, we could ship those in mediawiki-codesniffer as off by default.

If none of that's needed, that's obviously even better, but if it helps kick-start this transition, I think that would be fine :)

Note that phpcs ruleset is controllable per repository. The same way that we disable some rules in repositories not yet ready to follow a newer convention, it is also possible to enable additional rules on a per-repo bases.

I think the reason back then was that wikibase team maintains lots of repos instead of one or two (which is another can of worms) and if we want to, for example, enforce a different rule for our codebase, we have to change all phpcs files one by one.

My solution to this is to actually merge back libraries to our repos, which we has done in one or two cases already.

Note that phpcs ruleset is controllable per repository. The same way that we disable some rules in repositories not yet ready to follow a newer convention, it is also possible to enable additional rules on a per-repo bases.

I think the reason back then was that wikibase team maintains lots of repos instead of one or two (which is another can of worms) and if we want to, for example, enforce a different rule for our codebase, we have to change all phpcs files one by one.

I think we could solve that with a package anyway (having it use a hook to write a file into the correct place).
But having these rules that synced I believe is undesired.

Related: T164653: Review rules in wikibase/wikibase-codesniffer and see which are appropriate for MW-CS

I've suggested this elsewhere, but as a start, I think we can move the Wikibase ruleset into the mediawiki-codesniffer package, so we no longer need to maintain an extra dependent package in the middle. Wikibase repos would continue to point at the Wikibase ruleset via .phpcs.xml. This also would make changes to the MediaWiki ruleset immediately available to Wikibase repos. And then it should be more straightforward to merge stuff from the Wikibase ruleset into the main MediaWiki one, eventually sunsetting Wikibase.

Related tickets:

Back in 2017 I had multiple reasons to push for a custom rule set. Many of them are not valid any more:

  • Back then none of the Wikibase codebases on GitHub used the upstream MediaWiki rule set, but custom phpcs.xml, each heavily different (example). This was partly because one of the original developers disagreed with parts of the MediaWiki style guide. Introducing the Wikibase rule set was a way to first replace these different phpcs.xml with an accepted standard, and then slowly migrate towards the upstream standard.
  • It was a place where I was free to play around with custom rules, without having to convince people outside of the Wikidata team.
  • It allowed to introduce much stricter rules that won't be accepted upstream.
  • I realized that all Wikibase codebases excluded the same upstream sniffs that conflicted with the code style the Wikidata team agreed on. These exclusions are all documented and reasoned here: https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/ruleset.xml
  • I wanted to have a single place that allows to update all codebases at once. This didn't worked out great. This is mostly because the MediaWiki CodeSniffer project got much, much more active and pushed out releases much faster. Keeping up with this is expensive, and might not be worth it at all. Because of this I removed the Wiklibase CodeSniffer from many codebases already.

Reviewing some individual customizations:

This leaves only a few things:

TL;DR: I suggest to archive the WikibaseCodeSniffer.

So looking at the great analysis from @thiemowmde it looks like we could / should in order to be in the best position to get rid of wikbase-codesniffer:

I'll try and remember to bring this up in one of our next tech prioritization meetings

I just +2ed the last change mentioned in my above comment.
Which means we should be able to actually retire wikibase-codensiffer after the next release, which will either be v33 or v32.1