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

PHP 7.3 compatibility and testing #68

Merged
merged 9 commits into from Mar 22, 2019
Merged

Conversation

Ocramius
Copy link
Member

Fixes #64

Rebases #64 and introduces CI entries for PHP 5.6~7.3

@Ocramius Ocramius added this to the 1.3.2 milestone Mar 22, 2019
@Ocramius Ocramius self-assigned this Mar 22, 2019
@Ocramius Ocramius requested a review from Potherca March 22, 2019 08:29
@Ocramius
Copy link
Member Author

@Potherca @shivashanti @jacekkarczmarczyk @monojp can I ask you to glance over this? If it looks good, I'll go ahead and tag 1.4.0, create 1.4.x and merge into master afterwards.

@@ -23,13 +23,16 @@
}
],
"require": {
"php": ">=5.1.2"
"php": "^7.1.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@Ocramius can’t php be “^5.3.2|^7.0”?

Copy link
Contributor

Choose a reason for hiding this comment

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

PHP 5 is too old but this library users seems to have existing older servers.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but older versions would continue to run. This release is about fixing support for stable PHP versions.

If we don't test against a PHP version, we can't consider it "supported", hence the restriction in first place.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is no problem if the set to --ignore-platform-reqs

Copy link
Member Author

Choose a reason for hiding this comment

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

@tanakahisateru
Copy link
Contributor

    "version":"1.3.1",

can't be removed?

@Ocramius
Copy link
Member Author

@tanakahisateru I didn't clean up code in this patch - don't have time for it right now, and what's more pressing is the support for newer releases. Anyone willing to take on the endeavor of improving the code quality of PHPTAL is welcome to do so with further patches (better if we target 2.0 for those though)

Copy link
Contributor

@shivashanti shivashanti left a comment

Choose a reason for hiding this comment

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

it seems ok.

@Ocramius
Copy link
Member Author

Right, shipping this, but will not merge to 1.3.x: 1.4.x will be started, and I'll tag from there.

1.3.x is only for security issues from now on.

@Ocramius Ocramius changed the base branch from 1.3.x to 1.4.x March 22, 2019 09:17
@Ocramius Ocramius merged commit 37cf3cc into 1.4.x Mar 22, 2019
@Ocramius Ocramius deleted the fix/#64-php-7.3-compatibility branch March 22, 2019 09:17
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.

None yet

4 participants