Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PHP 8.2 support, drop PHP 7.4 #90

Merged
merged 4 commits into from
Sep 16, 2022
Merged

Add PHP 8.2 support, drop PHP 7.4 #90

merged 4 commits into from
Sep 16, 2022

Conversation

Slamdunk
Copy link
Contributor

No description provided.

@@ -297,6 +297,9 @@ public function testPHPBlock54()
{
if (version_compare(PHP_VERSION, '5.4.0') < 0) $this->markTestSkipped('PHP < 5.4');

ini_set('short_open_tag', 0);
if (ini_get('short_open_tag')) $this->markTestSkipped("PHP is buggy");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I can't get what testPHPBlock54 should test, but this seems now needed to get it pass

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Potherca do you perhaps know what's going on here?

tests/TalCommentTest.php Show resolved Hide resolved
classes/PHPTAL/Dom/Element.php Outdated Show resolved Hide resolved
classes/PHPTAL/Php/TalesChainExecutor.php Outdated Show resolved Hide resolved
classes/PHPTAL/RepeatController.php Show resolved Hide resolved
tests/TalConditionTest.php Outdated Show resolved Hide resolved
tests/SourceTest.php Outdated Show resolved Hide resolved
classes/PHPTAL/Tokenizer.php Outdated Show resolved Hide resolved
classes/PHPTAL/StringSource.php Outdated Show resolved Hide resolved
@Slamdunk
Copy link
Contributor Author

I admit I've done the minimal effort to get the tests pass because code quality without CSFix & SA tools is a pain.

I'm undecided between answering your change requests with "Ain't nobody got time for that" or "Ok but only with doctrine/coding-standard and psalm at level 6"

@Ocramius
Copy link
Member

@Slamdunk I would just make sure we improve what we can, without breaking BC. I've done hundreds of these patches on laminas/: we don't need to refactor anything particular, just the diff lines :)

classes/PHPTAL/Php/Attribute.php Outdated Show resolved Hide resolved
classes/PHPTAL/Php/Attribute/TAL/Attributes.php Outdated Show resolved Hide resolved
classes/PHPTAL/Php/Attribute/TAL/Attributes.php Outdated Show resolved Hide resolved
classes/PHPTAL/RepeatController.php Show resolved Hide resolved
classes/PHPTAL/Dom/PHPTALDocumentBuilder.php Outdated Show resolved Hide resolved
@Ocramius
Copy link
Member

I'll remove the 7.4 pipeline requirement once ready and releasable 👍

@Ocramius Ocramius self-assigned this Sep 16, 2022
@Ocramius Ocramius merged commit 1836dce into phptal:1.7.x Sep 16, 2022
@Ocramius
Copy link
Member

@Slamdunk Slamdunk deleted the php_82 branch September 26, 2022 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants