Re-enable codesniffer sniffs disabled in RelatedArticles
Closed, ResolvedPublic
Actions

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

	Subject	Repo	Branch	Lines +/-
	Restore default MediaWiki codesniffer configuration	mediawiki/extensions/RelatedArticles	master	+21 -18

Customize query in gerrit

Related Objects

Mentioned Here: T168384: Fix codesniffer sniffs disabled with the update to 0.9
T170624: Restore Skip ApiMobileViewTest::testView and verify PHP unit tests for RelatedArticles pass

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

pmiazga added a parent task: T162936: Improve developer happiness in reading web maintained extensions.Jul 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

gerritbot added a project: Patch-For-Review.Jul 13 2017, 3:23 PM

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 Web-Team-Backlog board.

Jdlrobson moved this task from Triaged but Future to Upcoming on the Web-Team-Backlog board.

Jdlrobson added a project: good first task.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJul 13 2017, 4:12 PM

pmiazga added a project: Readers-Web-Kanbanana-Board-Old.Jul 13 2017, 5:23 PM

pmiazga moved this task from To Do to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.

ovasileva moved this task from Upcoming to 2017-18 Q1 on the Web-Team-Backlog board.Jul 13 2017, 5:41 PM

Blocked on T170624

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.

pmiazga added projects: Readers-Web-Kanbanana-Board-Old, Unplanned-Sprint-Work.Jul 25 2017, 12:05 PM

Bringing it to current sprint as