Opened 5 years ago
Closed 5 years 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
@
5 years 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
@
5 years 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.
Ref: [46919]