Page MenuHomePhabricator

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

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.

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

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

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"

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 Readers-Web-Backlog board.
Aklapper updated the task description. (Show Details)Nov 20 2017, 10:47 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 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 updated the task description. (Show Details)

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

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

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

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

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

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

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

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

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

Pppery claimed this task.Dec 6 2017, 11:21 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

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.

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

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

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

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

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

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

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

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

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)

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

D3r1ck01 moved this task from Backlog to Doing on the good first task board.Jan 26 2018, 4:24 PM
Pppery closed this task as Resolved.Mar 5 2018, 5:58 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