search

totten / civix

CiviCRM Extension Builder
http://civicrm.org/
Other
56 stars 56 forks source link

Fix php notices #344

Closed colemanw closed 5 months ago

colemanw commented 5 months ago

Fixes test failures on max.

colemanw commented 5 months ago

Well crap I started this PR because of all the php warnings causing test fails and then realized you've already opened a similar PR #340 @totten can you please merge that other one so the tests can run?

totten commented 5 months ago

@colemanw To your main point, yes, civix does need these kinds of updates. And PR #340 is actually further along for doing them. And what I really want is for the test-matrix to show that #340 achieved its goal wrt to civix/php82. But it can't quite do that because it's wrapped up with the question of core's compatibility.

Put another way - even if (as I believe) #340 fixes compatibility for civix/php82, it still gets flagged as red because civix+cv support multiple versions of core (including core@5.57, which doesn't support php82). For testing downstream tools like cv/civix, the test-environments need permutations like:

(It's usually not that bad -- we go long periods keeping the requirements flat for many versions. But we're in a transitional period on core's requirements. There are others way to haggle -- maybe drop 5.57 (even though it's actually still compatible)? But 5.63 is probably no better. And the whole thing will recur. Usually, we get a good 6-18 months... but with current php83 chatter, I think we're looking a shorter turnaround here. I think it's better to fix the root problem -- supporting better permutations of testing core x php)