Page MenuHomePhabricator
Create Task
Maniphest T421071

Standardize line length enforcement in CheckUser
Closed, ResolvedPublic
Actions

Description

Background

During sprint planning, the team discussed line length standards for CheckUser. The existing linter configuration (inherited from mediawiki-codesniffer) sets a warning at 120 characters returning a CLI exit code, but the hard limit error is at 9999, creating ambiguity about what is required vs. optional in code reviews.

Problem

Without a documented and enforced standard, reviewers had no clear basis for when to raise or ignore line length comments, leading to inconsistent review feedback.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Fix lintingmediawiki/extensions/CheckUsermaster+14 -25
Fix lintingmediawiki/extensions/CheckUsermaster+1 -1
Add php-cs-fixer to enforce fully multiline argument listsmediawiki/extensions/CheckUsermaster+66 -27
Add EnsureFullyMultilineSniff for multiline arg listsmediawiki/tools/codesniffermaster+1 K -2
Add coding conventions, php-cs-fixer, and reformat arg listsmediawiki/extensions/CheckUsermaster+2 K -725
Change 120-char line length limit warning to errormediawiki/extensions/CheckUsermaster+123 -15
Customize query in gerrit

Event Timeline

MPostoronca-WMF created this task.Mar 24 2026, 1:41 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 24 2026, 1:41 PM
MPostoronca-WMF claimed this task.Mar 24 2026, 1:41 PM
MPostoronca-WMF moved this task from Backlog to In progress on the Product Safety and Integrity (Sprint Forsythia (Mar 23 - Apr 10))) board.
Dreamy_Jazz updated the task description. (Show Details)Mar 24 2026, 1:48 PM
Dreamy_Jazz subscribed.

The existing linter configuration (inherited from mediawiki-codesniffer) sets a soft warning at 120 characters but no hard error

mediawiki-codesniffer in PHP sets 120 chars as a hard limit, which you have to specifically ignore because otherwise the tests will fail in CI and prevent merging. Is this not PHP you are talking about?

gerritbot added a comment.Mar 24 2026, 2:02 PM

Change #1259980 had a related patch set uploaded (by Mpostoronca; author: Mpostoronca):

[mediawiki/extensions/CheckUser@master] Enforce 120-char line length limit in CheckUser

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

gerritbot added a project: Patch-For-Review.Mar 24 2026, 2:02 PM
MPostoronca-WMF added a comment.Mar 24 2026, 3:09 PM
In T421071#11743624, @Dreamy_Jazz wrote:

The existing linter configuration (inherited from mediawiki-codesniffer) sets a soft warning at 120 characters but no hard error

mediawiki-codesniffer in PHP sets 120 chars as a hard limit, which you have to specifically ignore because otherwise the tests will fail in CI and prevent merging. Is this not PHP you are talking about?

Yes, you're right that a lineLimit warning causes composer phpcs to fail, so in practice long lines do block CI unless suppressed with // phpcs:ignore Generic.Files.LineLength.TooLong .
However, mediawiki-codesniffer sets lineLimit=120 (a warning) and absoluteLineLimit=9999 (effectively no hard error).
So in essence phpcs:ignore Generic.Files.LineLength.TooLong suppresses only the warning, not an absolute error.

I've updated the description.

MPostoronca-WMF updated the task description. (Show Details)Mar 24 2026, 3:10 PM
gerritbot added a comment.Mar 25 2026, 4:10 PM

Change #1260745 had a related patch set uploaded (by Mpostoronca; author: Mpostoronca):

[mediawiki/extensions/CheckUser@master] Add coding conventions, php-cs-fixer, and reformat arg lists

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

gerritbot added a comment.Mar 25 2026, 5:18 PM

Change #1259980 abandoned by Mpostoronca:

[mediawiki/extensions/CheckUser@master] Change 120-char line length limit warning to error

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

gerritbot added a comment.Mar 26 2026, 12:29 PM

Change #1261386 had a related patch set uploaded (by Mpostoronca; author: Mpostoronca):

[mediawiki/tools/codesniffer@master] Add EnsureFullyMultilineSniff for multiline arg lists

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

gerritbot added a comment.Mar 30 2026, 12:31 PM

Change #1260745 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Add coding conventions, php-cs-fixer, and reformat arg lists

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

ReleaseTaggerBot added a project: MW-1.46-notes (1.46.0-wmf.22; 2026-03-31).Mar 30 2026, 1:00 PM
gerritbot added a comment.Mar 31 2026, 7:53 AM

Change #1261386 abandoned by Mpostoronca:

[mediawiki/tools/codesniffer@master] Add EnsureFullyMultilineSniff for multiline arg lists

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

Maintenance_bot removed a project: Patch-For-Review.Mar 31 2026, 8:32 AM
gerritbot added a comment.Mar 31 2026, 9:37 AM

Change #1265373 had a related patch set uploaded (by Mpostoronca; author: Mpostoronca):

[mediawiki/extensions/CheckUser@master] Add php-cs-fixer to enforce fully multiline argument lists

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

gerritbot added a project: Patch-For-Review.Mar 31 2026, 9:37 AM
gerritbot added a comment.Mar 31 2026, 10:43 AM

Change #1265373 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Add php-cs-fixer to enforce fully multiline argument lists

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

ReleaseTaggerBot edited projects, added: MW-1.46-notes (1.46.0-wmf.23; 2026-04-07); removed: MW-1.46-notes (1.46.0-wmf.22; 2026-03-31).Mar 31 2026, 11:00 AM
Maintenance_bot removed a project: Patch-For-Review.Mar 31 2026, 11:31 AM
Dreamy_Jazz moved this task from In progress to Needs review on the Product Safety and Integrity (Sprint Forsythia (Mar 23 - Apr 10))) board.Mar 31 2026, 11:58 AM
Dreamy_Jazz moved this task from Needs review to Done on the Product Safety and Integrity (Sprint Forsythia (Mar 23 - Apr 10))) board.
Dreamy_Jazz closed this task as Resolved.Apr 8 2026, 9:02 AM
gerritbot added a comment.Apr 8 2026, 2:04 PM

Change #1268973 had a related patch set uploaded (by Mpostoronca; author: Mpostoronca):

[mediawiki/extensions/CheckUser@master] Fix linting

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

gerritbot added a project: Patch-For-Review.Apr 8 2026, 2:04 PM
gerritbot added a comment.Apr 8 2026, 2:11 PM

Change #1268973 abandoned by Mpostoronca:

[mediawiki/extensions/CheckUser@master] Fix linting

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

gerritbot added a comment.Apr 8 2026, 2:13 PM

Change #1268977 had a related patch set uploaded (by Mpostoronca; author: Mpostoronca):

[mediawiki/extensions/CheckUser@master] Fix linting

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

gerritbot added a comment.Apr 8 2026, 2:39 PM

Change #1268977 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Fix linting

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

ReleaseTaggerBot edited projects, added: MW-1.46-notes (1.46.0-wmf.24; 2026-04-14); removed: MW-1.46-notes (1.46.0-wmf.23; 2026-04-07).Apr 8 2026, 3:00 PM
Maintenance_bot removed a project: Patch-For-Review.Apr 8 2026, 3:31 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 · Credits