WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 2 months ago

#48966 closed task (blessed) (wontfix)

Add PHPCompatibility check to all branches

Reported by: johnbillion Owned by: desrosj
Milestone: Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords:
Focuses: coding-standards Cc:

Description

In #46152 the PHPCompatibility library was added. This should be backported to all previous branches, with appropriate PHP version constraints so branches prior to 5.2 don't end up with code that requires PHP > 5.2.

Change History (3)

#2 @desrosj
3 months ago

  • Owner set to desrosj
  • Status changed from new to assigned

Great idea, @johnbillion. I can tackle this when working on #48301.

Before 5.0, Core did not have a composer.json file. I can't think of any arguments against introducing this file in those branches, but I wanted to pose the question for others to weigh in on.

#3 @desrosj
2 months ago

  • Keywords needs-patch removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

I did some experimenting with this today. Here are my findings.

I ran the compatibility checks on two branches: 5.2 and 4.1.

5.2: Constraints of PHP 5.6-7.3 returned the following:

A TOTAL OF 88 SNIFF VIOLATIONS WERE FOUND IN 18 SOURCES

4.1: Constraints of PHP 5.2-5.6 returned the following:

A TOTAL OF 162 SNIFF VIOLATIONS WERE FOUND IN 38 SOURCES

When [46290] was committed, it included some false positive fixes (and still is not passing). Many of those false positives exist in earlier branches, so simply backporting is not enough to make this work. We could work to adjust the rule set to exclude certain tests and achieve a passed compatibility scan, but that defeats the purpose of adding a compatibility scan. We could also add the scan to the allowed failures in Travis (similar to trunk and 5.3), but that will also defeat the purpose because realistically no one will look at the summary of that job and compare the scan results.

Fixing these would create churn (38 files in the 4.1 branch, for example) which would greatly increase the size of the next security update for the older branches.

While I love the idea, I am going to close this out as a wontfix because I don't think this is worth adding the additional risk to the next auto update for older branches or the required level of effort. If someone else can justify the level of effort here and would like to work on this, feel free to reopen with an explanation and tackle.

Note: See TracTickets for help on using tickets.