Page MenuHomePhabricator

Release a new version of WikibaseCodeSniffer
Closed, ResolvedPublic

Description

In order to be able to achieve T192167: Upgrade PHPUnit from 4/6 to 8, we need to enforce the use of new functions in tests. For that, we need to apply mediawiki-codesniffer 29.0.0 to Wikidata extensions, which would require a new version of WikibaseCodeSniffer.

Event Timeline

MaxSem created this task.Jan 21 2020, 4:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 21 2020, 4:47 PM

CC @Ladsgroup as releaser of the last two versions and (AFAICS) most active maintainer.

Note: I was the driving force when we introduced the WikibaseCodeSniffer rule set. The main motivations back then:

  1. Being able to experiment with additional, custom sniffs that neither exist in the MediaWiki nor the PHPCS rulesets. However, a bunch of these custom sniffs became obsolete by now, or have been incorporated into the MediaWiki ruleset. There are a few custom sniffs left. I'm not sure if the value created by these extra sniffs is big enough to justify having the WikibaseCodeSniffer codebase.
  2. Being able to enable additional sniffs by default for all Wikibase codebases, without the need to repeat the configuration in every .phpcs.xml file.
  3. Being able to get rid of some of the less valuable MediaWiki rules in all Wikibase codebases, without the need to repeat configuration. The motivation for each exception is documented in https://github.com/wmde/WikibaseCodeSniffer/blob/master/Wikibase/ruleset.xml.

If the current Wikidata team(s) don't find any of this valuable any more, they should feel free to ditch the WikibaseCodeSniffer and use the MediaWiki-Codesniffer directly.

If there's still value in having separate Wikibase configuration (I don't think so, but that's just me), then I think we could ship it inside the MediaWiki-Codesniffer repo so it doesn't fall behind at least.

Addshore triaged this task as Medium priority.Jan 23 2020, 7:40 AM
Addshore moved this task from Incoming to Needs Work on the Wikidata-Campsite board.

I would be in favor of reducing the code base we maintain and reducing the overhead of maintaining those (upgrading libraries, etc.). I say let's ditch it in favor of mediawiki code sniffer.

I think we could ship it inside the MediaWiki-Codesniffer repo so it doesn't fall behind at least.

I think this is a great idea to get started with. I guess the plan would be to:

  • Export WB sniffs to MW-CS (the ones that we want to keep)
  • Move the WB ruleset to MW-CS
  • Archive the WB-CS repo
  • For each repo using WB-CS:
    • composer: remove WB-CS, add MW-CS
    • .phpcs.xml: load the WB ruleset from the new location

And how are we going to resolve all the failures we’ll get once we no longer ignore the MW-CS rules that WB-CS currently disables? I think that bullet point is missing from the list above. (Unless “Move the WB ruleset to MW-CS” includes disabling those rules for all of MW-CS?)

(Unless “Move the WB ruleset to MW-CS” includes disabling those rules for all of MW-CS?)

Sort of, yes. I believe it's possible to have two different rulesets, and choose which one to use. WB repos would still point to the same ruleset, just at another location.

There is a .phpcs.xml in Wikibase. You can move the exclusions you want to keep to this file, including the comments explaining them.

Well, presumably we use WB-CS in more than one repo, so we’d have to duplicate those exclusions again :/

Not if we move the main ruleset file per T243296#5826268. And, of course, unless it turns out to be impossible to have two different rulesets in MW-CS.

Well, presumably we use WB-CS in more than one repo, so we’d have to duplicate those exclusions again :/

These exclusions are (very likely) technical debt that we need to pay sooner or later. Also other repos that are not Wikibase are rather small and I doubt they would need all exclusions.

Addshore closed this task as Resolved.Jan 24 2020, 3:31 PM
Addshore claimed this task.
Addshore added a subscriber: Addshore.

1.1.0 just released and has:

Upgrade PHPUnit from ^4.8.35 to ^8.4
Upgrade MediaWiki Code Sniffer from v28 to v29

I'm going to go around and update this in various places that it is used.
Also going forward we will consider if we even want this seperate library or if this should be merged into the mediawiki bit

Restricted Application added a project: User-Addshore. · View Herald TranscriptJan 24 2020, 3:31 PM

I'm going to go around and update this in various places that it is used.

I can help with review.

Also going forward we will consider if we even want this seperate library or if this should be merged into the mediawiki bit

Note that there might be two separate discussions: 1-whether we want a separate *repository*, 2-whether we want a separate *ruleset*. We can merge the repositories while keeping a standalone config for WB, which IMHO requires less discussion and is a good idea to make it happen.