Page MenuHomePhabricator
Create Task
Maniphest T169483

[Wikispeech] Investigate excluded code sniffs
Closed, ResolvedPublic2 Estimated Story Points
Actions

Description

With https://gerrit.wikimedia.org/r/360291 and https://gerrit.wikimedia.org/r/362785 Wikispeech got mediawiki-codesniffer bumped fopm 0.7.2 to 0.10.0. The upgrade however required that certain sniffs were excluded/deactivated for it to pass.

Investigate what blocks the sniffs and where appropriate amend and reactivate.

		<exclude name="MediaWiki.Commenting.FunctionComment.MissingParamName" />
		<exclude name="MediaWiki.Commenting.FunctionComment.MissingParamTag" />
		<exclude name="MediaWiki.Commenting.FunctionComment.ParamNameNoMatch" />
		<exclude name="MediaWiki.Commenting.FunctionComment.SpacingAfter" />
		<exclude name="MediaWiki.Commenting.FunctionComment.MissingDocumentationProtected" />
		<exclude name="MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic" />

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Re-enable disabled sniffsmediawiki/extensions/Wikispeechmaster+27 -45
Customize query in gerrit

Related Objects

Event Timeline

Lokal_Profil created this task.Jul 3 2017, 6:46 AM
Lokal_Profil moved this task from Backlog to Technical debt on the Wikispeech-WMSE board.
Lokal_Profil updated the task description. (Show Details)Jul 3 2017, 6:50 AM
Lokal_Profil updated the task description. (Show Details)Jul 4 2017, 8:14 AM
Lokal_Profil edited projects, added Wikispeech (Sprint 2017-07-05), User-Sebastian_Berlin-WMSE; removed Wikispeech.Jul 4 2017, 10:00 AM
Lokal_Profil set the point value for this task to 2.

All of the excluded sniffs are related to documentation/commenting so it should be doable to fix them.

Lokal_Profil updated the task description. (Show Details)Jul 4 2017, 10:00 AM
gerritbot subscribed.Jul 5 2017, 8:06 AM

Change 363292 had a related patch set uploaded (by Lokal Profil; owner: Lokal Profil):
[mediawiki/extensions/Wikispeech@master] Re-enable disabled sniffs

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

gerritbot added a project: Patch-For-Review.Jul 5 2017, 8:07 AM
Lokal_Profil mentioned this in rEWISa3ba687d2dbe: Re-enable disabled sniffs.Jul 5 2017, 8:09 AM
Lokal_Profil mentioned this in rEWISc36cd36791d7: Re-enable disabled sniffs.Sep 2 2017, 9:52 AM
Lokal_Profil mentioned this in rEWISe53f877c3aee: Re-enable disabled sniffs.Sep 2 2017, 10:31 AM
Lokal_Profil mentioned this in rEWISfe34de01efd2: Re-enable disabled sniffs.
Lokal_Profil moved this task from Backlog to In progress on the Wikispeech (Sprint 2017-07-05) board.Sep 2 2017, 10:33 AM

The newest patch re-activates all sniffs other than those relating to the newest rules on one class per file (with the same name).

Lokal_Profil added a project: User-Lokal_Profil.Sep 2 2017, 10:33 AM
Lokal_Profil moved this task from 📥 Backlog to 🔬 to Review on the User-Lokal_Profil board.
Lokal_Profil claimed this task.Sep 2 2017, 10:55 AM
gerritbot added a comment.Sep 2 2017, 10:56 AM

Change 363292 merged by jenkins-bot:
[mediawiki/extensions/Wikispeech@master] Re-enable disabled sniffs

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

Stashbot subscribed.Sep 2 2017, 12:33 PM

Mentioned in SAL (#wikimedia-cloud) [2017-09-02T12:33:18Z] <Lokal_Profil> Deploy latest from Git master: fe34de0 (T169483)

Kizule closed this task as Resolved.Sep 2 2017, 1:15 PM
Jopparn moved this task from Backlog to Done on the User-Sebastian_Berlin-WMSE board.Oct 9 2017, 8:09 AM
Sebastian_Berlin-WMSE added a comment.Oct 26 2017, 4:27 PM

I just noticed that phpc(s|bf) runs on a lot of files (my guess is the whole vendor directory) when I run it in my Vagrant installation. There is an exclude-pattern element in .phpcs.xml, but that doesn't seem work as intended.

Lokal_Profil added a comment.EditedOct 27 2017, 8:45 PM
In T169483#3712761, @Sebastian_Berlin-WMSE wrote:

I just noticed that phpc(s|bf) runs on a lot of files (my guess is the whole vendor directory) when I run it in my Vagrant installation. There is an exclude-pattern element in .phpcs.xml, but that doesn't seem work as intended.

On gerrit it's only checking 14 files so that seems to be an issue in how Vagrant is running it. Checking on my Vagrant it also only sniffs 14 files.

Are you running it through composer test? If I just run php -p -s then yes it sniffs 2497 files.

Lokal_Profil moved this task from 🔬 to Review to ☑️ Done on the User-Lokal_Profil board.Jan 22 2018, 9:31 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits