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.

Sign up for GitHub

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

Jump to bottom

PHP 8.0 support #20

Merged
merged 3 commits into from Jan 1, 2021
Merged

PHP 8.0 support #20

merged 3 commits into from Jan 1, 2021

Conversation

svycka
Copy link
Contributor

@svycka svycka commented Oct 21, 2020

closes #19

@svycka svycka force-pushed the feature-19/php8.0 branch 2 times, most recently from a87f43c to bba27d5 Compare October 21, 2020 11:56
@svycka svycka marked this pull request as draft October 21, 2020 11:57
@svycka svycka force-pushed the feature-19/php8.0 branch 3 times, most recently from cb62904 to c05938d Compare October 21, 2020 12:59
@boesing boesing added Enhancement hacktoberfest-accepted Issues/Pull-Requests which can be fixed during Hacktoberfest: https://hacktoberfest.digitalocean.com labels Oct 21, 2020
@boesing boesing added this to In progress in PHP 8.0 via automation Oct 21, 2020
@boesing boesing removed the hacktoberfest-accepted Issues/Pull-Requests which can be fixed during Hacktoberfest: https://hacktoberfest.digitalocean.com label Oct 21, 2020
@svycka svycka force-pushed the feature-19/php8.0 branch 4 times, most recently from ddab630 to 167c8d5 Compare October 22, 2020 06:26
@svycka svycka marked this pull request as ready for review October 22, 2020 06:28
@svycka
Copy link
Contributor Author

svycka commented Oct 22, 2020

@boesing okay all green

svycka
svycka commented Oct 22, 2020
View reviewed changes
composer.json Show resolved Hide resolved
@svycka
Copy link
Contributor Author

svycka commented Oct 23, 2020

@boesing then I think it's ready for review

froschdesign
froschdesign reviewed Oct 23, 2020
View reviewed changes
.travis.yml Outdated Show resolved Hide resolved
@svycka svycka force-pushed the feature-19/php8.0 branch 2 times, most recently from 8b81c87 to 70c29f3 Compare October 23, 2020 07:11
froschdesign
froschdesign reviewed Oct 23, 2020
View reviewed changes
src/Encrypt/Openssl.php Outdated
* Free key resource if necessary.
* PHP 8 automatically frees the key instance and deprecates the function
*/
private function freeKeyResource($keys): void
Copy link
Member

Choose a reason for hiding this comment

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

The DocBlock for $keys is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true 👍

Copy link
Member

@boesing boesing Oct 31, 2020 •
edited

Choose a reason for hiding this comment

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

Please add proper native typehint for keys here aswell as phpdoc to declare what keys are (I expect array<int,string> here.

boesing
boesing requested changes Oct 31, 2020
View reviewed changes
Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

Huge improvements here! Thanks!
I've added some change requests. If applied, we are ready to merge 👍

src/Encrypt/Openssl.php Outdated
* Free key resource if necessary.
* PHP 8 automatically frees the key instance and deprecates the function
*/
private function freeKeyResource($keys): void
Copy link
Member

@boesing boesing Oct 31, 2020 •
edited

Choose a reason for hiding this comment

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

Please add proper native typehint for keys here aswell as phpdoc to declare what keys are (I expect array<int,string> here.

src/Encrypt/Openssl.php Show resolved Hide resolved
PHP 8.0 automation moved this from In progress to Review in progress Oct 31, 2020
@svycka svycka requested a review from boesing November 3, 2020 06:42
PHP 8.0 automation moved this from Review in progress to Reviewer approved Nov 17, 2020
boesing
boesing approved these changes Nov 17, 2020
View reviewed changes
Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

LGTM.

@boesing
Copy link
Member

boesing commented Dec 22, 2020

Trying to create a release next week.
Sorry for having you waiting...

@svycka
laminas#19 PHP 8.0 support
8a0226c 
Signed-off-by: Vytautas Stankus <svycka@gmail.com>
svycka
svycka commented Dec 23, 2020
View reviewed changes
.travis.yml Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
boesing
boesing approved these changes Jan 1, 2021
View reviewed changes
Copy link
Member

@boesing boesing left a comment

Choose a reason for hiding this comment

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

LGTM

@boesing
qa: rename method as it is freeing multiple resources
9f5cd34 
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing
qa: add CHANGELOG entry
7dece98 
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@boesing boesing merged commit 87ebc8b into laminas:2.10.x Jan 1, 2021
PHP 8.0 automation moved this from Reviewer approved to Done Jan 1, 2021
@boesing
Copy link
Member

boesing commented Jan 1, 2021

Thanks, @svycka!

@boesing boesing added this to the 2.10.0 milestone Jan 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boesing boesing boesing approved these changes

@froschdesign froschdesign Awaiting requested review from froschdesign

Assignees
No one assigned
Labels
Enhancement
Projects
No open projects
PHP 8.0
  
Done
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.

PHP 8.0 support
3 participants
@svycka @boesing @froschdesign