Page MenuHomePhabricator

Re-enable codesniffer sniffs disabled in RelatedArticles
Closed, ResolvedPublic

Description

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

MediaWiki.Commenting.FunctionComment.MissingParamComment
MediaWiki.Commenting.FunctionComment.MissingParamTag
MediaWiki.Commenting.FunctionComment.ParamNameNoMatch
MediaWiki.Commenting.FunctionComment.MissingDocumentationPublic

Runing phpcs without those sniffs gives 30 errors and 1 warning in 4 files.

Plan (YMMV)

  • Enable sniffs one by one and send patches fixing the problems

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

Related Gerrit Patches:
mediawiki/extensions/RelatedArticles : masterRestore default MediaWiki codesniffer configuration

Event Timeline

pmiazga created this task.Jul 13 2017, 3:12 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 13 2017, 3:12 PM

Change 365042 had a related patch set uploaded (by Pmiazga; owner: Pmiazga):
[mediawiki/extensions/RelatedArticles@master] Restore default MediaWiki codesniffer configuration

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

pmiazga claimed this task.Jul 13 2017, 3:23 PM
pmiazga updated the task description. (Show Details)
Jdlrobson triaged this task as Low priority.Jul 13 2017, 4:10 PM
Jdlrobson moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
Jdlrobson moved this task from Triaged but Future to Upcoming on the Readers-Web-Backlog board.
Jdlrobson added a project: good first task.
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJul 13 2017, 4:12 PM
Jdlrobson moved this task from 2017-18 Q1 to Upcoming on the Readers-Web-Backlog board.

I believe this was unplanned (pmiazga added a project: Reading-Web-Kanban-Board.Thu, Jul 13, 10:23 AM). I'm hoping for us to do some other work in the RelatedPages next sprint so I think this can wait till then - fixing Popups was the goal for this sprint . Hope thats okay. It also buys us time to understand and fix the Minerva test issue in T170624.

@Jdlrobson - no problem. I brought this into the sprint while I was working on T168384 - My initial thought was to fix all phpcs issues but there were too many in different extensions. I created phab tickets to fix other extensions, and I fixed the RelatedPages issues as they were just couple of them plus they were easy to write.

Bringing it to current sprint as

  • the task is written and reviewed, ready for sign off
  • we used this task to verify failing Minerva PHPUnit tests

Signoff procedure: run phpcs and check it doesn't produce errors nor warnings. Verify phpcs.xml file has no disabled rules.

phuedx removed pmiazga as the assignee of this task.Jul 25 2017, 4:56 PM

Change 365042 merged by Jdlrobson:
[mediawiki/extensions/RelatedArticles@master] Restore default MediaWiki codesniffer configuration

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

Jdlrobson closed this task as Resolved.Jul 25 2017, 6:41 PM
Jdlrobson claimed this task.
Jdlrobson removed a project: Patch-For-Review.