Page MenuHomePhabricator
Create Task
Maniphest T170583

Re-enable codesniffer sniffs disabled in PageImages
Closed, ResolvedPublic2 Estimated Story Points
Actions

Description

PHPCodeSniffer helps us stick to same coding standards across MediaWiki and it's extensions. Currently PageImages phpcs config differs from the MediaWiki one - the following sniffs are disabled:

  • MediaWiki.Commenting.FunctionComment.MissingParamComment
  • MediaWiki.Commenting.FunctionComment.MissingParamTag
  • MediaWiki.Commenting.FunctionComment.MissingReturn
  • MediaWiki.Commenting.FunctionComment.ParamNameNoMatch
  • MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic
  • MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName
  • MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment
  • MediaWiki.Files.ClassMatchesFilename.NotMatch" />
  • MediaWiki.Files.ClassMatchesFilename.WrongCase" />
  • MediaWiki.Files.OneClassPerFile.MultipleFound" />

Runing phpcs without those sniffs gives 47 errors and 3 warnings in 8 files.

Google Code In

@Jdlrobson will mentor this.
The task should be timeboxed till 2-3hrs - work on re-enabling one sniff at a time. If you exceed the timebox push what you managed - every little bit helps!

To get setup install https://github.com/squizlabs/PHP_CodeSniffer globally and then run the following inside the MobileFrontend repo

phpcs -p -s

They should all pass as we are excluding certain rules inside .phpcs.xml

Plan (YMMV)

  • Remove a sniff from the .phpcs.xml file, run composer phpcs or phpcs -p -s, get a list of violations, provide patches in Wikimedia Gerrit to fix the violations. If you fix all violations for one sniff, also commit the removal of the exclusion from .phpcs.xml too.

AC

  • As many of the sniffs are made to pass as is possible.
  • If a sniff can't be made to pass for some reason, then document it as close as possible to the line disabling the sniff.

Details

SubjectRepoBranchLines +/-
Fix remaining php sniffsmediawiki/extensions/PageImagesmaster+37 -28
Re-enable "MediaWiki.Commenting.FunctionComment.MissingParamComment" sniffmediawiki/extensions/PageImagesmaster+33 -31
Re-enable "MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName" sniffmediawiki/extensions/PageImagesmaster+4 -5
Re-enable "MediaWiki.Commenting.FunctionComment.MissingReturn" sniffmediawiki/extensions/PageImagesmaster+1 -2
Partially fix class-file name mismatch sniffsmediawiki/extensions/PageImagesmaster+9 -18
Re-enable "MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic" sniffmediawiki/extensions/PageImagesmaster+35 -1
Re-enable "MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment" sniffmediawiki/extensions/PageImagesmaster+6 -5
Set up phpcs to detect all code style violationsmediawiki/extensions/PageImagesmaster+26 -2
Customize query in gerrit

Related Objects
Search...

Event Timeline

pmiazga created this task.Jul 13 2017, 2:53 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 13 2017, 2:53 PM
pmiazga added a parent task: T162936: Improve developer happiness in reading web maintained extensions.Jul 13 2017, 2:53 PM
gerritbot subscribed.Jul 13 2017, 2:54 PM

Change 365028 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/PageImages@master] Set up phpcs to detect all code style violations

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

gerritbot added a project: Patch-For-Review.Jul 13 2017, 2:55 PM
gerritbot added a comment.Jul 13 2017, 2:58 PM

Change 365028 abandoned by Pmiazga:
Set up phpcs to detect all code style violations

Reason:
I had old master branch, this repo already has a phpcs configuration

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

pmiazga closed this task as Declined.Jul 13 2017, 2:58 PM

I declined this task as I had old master. After pulling latest changes I see phpcs setup.

pmiazga renamed this task from Create PHPCS configuration for PageImages extension to Re-enable codesniffer sniffs disabled in PageImages.Jul 13 2017, 3:04 PM
pmiazga reopened this task as Open.
pmiazga removed a project: Patch-For-Review.
pmiazga updated the task description. (Show Details)

Restoring this task as existing phpcs config has many sniffs disabled - restored as "re-enable sniffs"

Jdlrobson moved this task from Incoming to Triaged but Future on the Web-Team-Backlog board.Jul 13 2017, 3:10 PM
pmiazga updated the task description. (Show Details)Jul 13 2017, 3:28 PM
Jdlrobson triaged this task as Low priority.Jul 13 2017, 4:11 PM
Jdlrobson added a project: good first task.
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJul 13 2017, 4:11 PM
Jdlrobson raised the priority of this task from Low to Medium.Aug 17 2017, 3:09 PM
Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.
Jdlrobson moved this task from Upcoming to Triaged but Future on the Web-Team-Backlog board.Aug 24 2017, 2:04 PM
Jdlrobson added a project: Africa-Wikimedia-Developers.Aug 28 2017, 3:14 PM
xSavitar moved this task from Backlog to New Contributor's Tasks (Incoming) on the Africa-Wikimedia-Developers board.Aug 28 2017, 9:13 PM
Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Sep 28 2017, 9:16 PM
Jdlrobson added a project: Google-Code-in-2017.Oct 16 2017, 10:14 PM
Jdlrobson moved this task from Upcoming to Triaged but Future on the Web-Team-Backlog board.Oct 31 2017, 12:58 AM
ovasileva moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Nov 14 2017, 10:54 AM
Aklapper updated the task description. (Show Details)Nov 20 2017, 10:47 PM
Aklapper added a subscriber: Jdlrobson.Nov 20 2017, 10:51 PM

@Jdlrobson: Any guestimation into how many Google-Code-in-2017 tasks this ticket should be broken down ("about 2-3 hours for an experienced contributor")? And explicitly asking: Do you plan to mentor these?

Aklapper added a parent task: T177810: Fix up PHPCS violations that are excluded in .phpcs.xml.Nov 20 2017, 11:02 PM
Aklapper updated the task description. (Show Details)Nov 20 2017, 11:06 PM
ovasileva set the point value for this task to 2.Nov 21 2017, 5:44 PM
Jdlrobson moved this task from Upcoming to Triaged but Future on the Web-Team-Backlog board.Nov 21 2017, 7:19 PM
Jdlrobson updated the task description. (Show Details)
Aklapper moved this task from Proposed tasks to Imported in GCI Site on the Google-Code-in-2017 board.Nov 29 2017, 4:40 PM

Imported as https://codein.withgoogle.com/tasks/5962093603848192/ (but not published yet)

gerritbot added a comment.Dec 6 2017, 9:07 PM

Change 395838 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.Commenting.FunctionComment.MissingParamComment" sniff

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

gerritbot added a project: Patch-For-Review.Dec 6 2017, 9:07 PM
gerritbot added a comment.Dec 6 2017, 9:49 PM

Change 395848 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.Commenting.FunctionComment.MissingReturn" sniff

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

gerritbot added a comment.Dec 6 2017, 10:02 PM

Change 395865 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment" sniff

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

gerritbot added a comment.Dec 6 2017, 10:12 PM

Change 395868 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName" sniff

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

gerritbot added a comment.Dec 6 2017, 10:17 PM

Change 395865 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment" sniff

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

gerritbot added a comment.Dec 6 2017, 10:48 PM

Change 395875 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic" sniff

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

gerritbot added a comment.Dec 6 2017, 10:53 PM

Change 395875 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic" sniff

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2017-12-12 (1.31.0-wmf.12)).Dec 6 2017, 11:00 PM
Pppery claimed this task.Dec 6 2017, 11:21 PM
gerritbot added a comment.Dec 6 2017, 11:53 PM

Change 395889 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/PageImages@master] Partially fix class-file name mismatch sniffs

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

Pppery added a comment.Dec 7 2017, 12:00 AM

Hit three-hour limit. There are three disabled sniffs remaining:

MediaWiki.Files.ClassMatchesFilename.NotMatch (partially fixed)
MediaWiki.Files.OneClassPerFile.MultipleFound (partially fixed)
MediaWiki.Files.ClassMatchesFilename.WrongCase

The first two sniffs are both caused by the same piece of code used in testing a protected method.

gerritbot added a comment.Dec 7 2017, 7:04 AM

Change 395889 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Partially fix class-file name mismatch sniffs

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

gerritbot added a comment.Dec 7 2017, 1:35 PM

Change 395848 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.Commenting.FunctionComment.MissingReturn" sniff

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

gerritbot added a comment.Dec 7 2017, 2:06 PM

Change 395868 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName" sniff

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

gerritbot added a comment.Dec 7 2017, 8:03 PM

Change 395838 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Re-enable "MediaWiki.Commenting.FunctionComment.MissingParamComment" sniff

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

xSavitar removed a project: Africa-Wikimedia-Developers.Dec 9 2017, 1:37 PM
xSavitar subscribed.

Removing Africa-Wikimedia-Developers tag as it's done in Google-Code-in-2017.

Pppery added a comment.Dec 9 2017, 3:47 PM

not fully done, there are still three sniff violations remaining

Jdlrobson updated the task description. (Show Details)Dec 21 2017, 4:10 AM
Jdlrobson updated the task description. (Show Details)
gerritbot added a comment.Dec 21 2017, 4:50 AM

Change 399559 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/PageImages@master] Fix remaining php sniffs

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

Pppery removed a parent task: T162936: Improve developer happiness in reading web maintained extensions.Dec 21 2017, 3:38 PM
xSavitar moved this task from Backlog to Doing on the good first task board.Jan 26 2018, 4:24 PM
Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.Feb 6 2018, 5:19 PM
Jdlrobson added projects: Africa-Wikimedia-Developers, Readers-Web-Kanbanana-Board-Old, Unplanned-Sprint-Work.Mar 2 2018, 10:17 PM

https://gerrit.wikimedia.org/r/399559 < would be great to get this merged and out of the way.

Jdlrobson moved this task from To Do to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Mar 2 2018, 10:17 PM
xSavitar moved this task from New Contributor's Tasks (Incoming) to Tasks in progress (Doing) on the Africa-Wikimedia-Developers board.Mar 3 2018, 3:57 PM
Pppery closed this task as Resolved.Mar 5 2018, 5:58 PM
gerritbot added a comment.Mar 5 2018, 6:07 PM

Change 399559 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Fix remaining php sniffs

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

pmiazga updated the task description. (Show Details)Mar 5 2018, 6:54 PM
xSavitar moved this task from Tasks in progress (Doing) to Technical Tasks (Completed) on the Africa-Wikimedia-Developers board.Mar 5 2018, 7:43 PM
ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2018-03-06 (1.31.0-wmf.24)); removed MW-1.31-release-notes (WMF-deploy-2017-12-12 (1.31.0-wmf.12)).Mar 6 2018, 3:01 PM
• Phabricator_maintenance moved this task from Upcoming to 2017-18 Q3 on the Web-Team-Backlog board.Mar 7 2018, 1:00 AM
xSavitar removed a project: Africa-Wikimedia-Developers.Jul 21 2018, 4:53 PM
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