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

Support maintained PHP Versions & introduce Rector #242

Closed
wants to merge 16 commits into from

Conversation

boekkooi-lengoo
Copy link

Hello,
First of thank you very much for maintaining this library! It saved me a ton of time and effort!

This PR is in bumping the support PHP version for this library to PHP 7.4 to match the currently support PHP versions (commit 733be54 and ae0ceb4).

After the PHP version update I introduced rector to help update and tweak the code based.
This also involved some manual changes and I have already fix some wrong types and other things.

I understand that this PR maybe way to big but I at least wanted to share my work even if its just for inspiration.
Thanks again for your work on this library!

Cheers,
Warnar

As of the 6 Dec 2021 PHP 7.3 is EOL.
For currently supported versions see: https://www.php.net/supported-versions.php
"Rector instantly upgrades and refactors the PHP code of your application." see https://github.com/rectorphp/rector

Using rector will enable less manual work while upgrading.
This adds the PHP rule sets to rector configuration.
See https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#php74 etc. for the added rules.
This applies the current rector configuration.
In this case it add the PHP rule sets see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#php74 etc. for the applied rules.
This adds the early return sets to rector configuration.
Enabling this allows us to improve the code and not have `else` statements everywhere.
See https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#earlyreturn for the added rules.
This adds the PHP type declaration rule sets to rector configuration which helps set type declarations etc.
See https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#typedeclaration etc. for the added rules.
This applies the PHP type declaration rule sets to rector configuration which helps set type declarations etc.
See https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#typedeclaration for the applied rules.

In order to apply the rule set correctly it was necessary change some methods in order to ensure Rector understand them correctly.
This adds the PHP type declaration rule strict sets to rector configuration which helps ensure we use the types we declare correctly.
See https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#strict for the added rules.
This applies the PHP type declaration strict rule sets to rector configuration which helps set type declarations etc.
See https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#strict for the applied rules.
Since headers are always parsed they do not need ot be nullable.
The property must match its parent.
The defined types seem to be incorrect and case `Return value must be of type ?` exceptions to be thrown.
@Webklex
Copy link
Owner

Webklex commented Jun 30, 2022

Hi @boekkooi-lengoo ,
wow thanks a lot for your pull request!

I'll check it out and probably merge it this weekend :)

Best regards,

@Webklex
Copy link
Owner

Webklex commented Jul 4, 2022

Hi @boekkooi-lengoo ,
I'm currently not sure about the potential implication of requiring rector. I know to little about it to blindly merge it. Especially the potential overhead. Guess I have to do some more reading.

Best regards,

@boekkooi-lengoo
Copy link
Author

boekkooi-lengoo commented Jul 4, 2022

Hey @Webklex,

I totally understand and I would myself also not require Rector. I used the tool to help improve the code base automatically instead of manually. This is also the reason why I did not add a rector dry-run to the GitHub Actions workflow.

Thanks for taking the time looking at the PR 😄

We can't return an Event object for the string given since it's using a static `dispatch` method.
Before the data was erased now it's not.
Based on Webklex#233
Webklex added a commit that referenced this pull request Jan 2, 2023
@Webklex
Copy link
Owner

Webklex commented Jan 2, 2023

Hi @boekkooi-lengoo ,
unfortunately I wasn't able to merge your pull request. However I would like to acknowledge your work and your suggestions which indeed influenced my changes. Please feel free to simply push a " " or "." - I'll happily merge it so you'll get listed as a contributor.

Best regards and a happy new year,

@boekkooi-lengoo
Copy link
Author

boekkooi-lengoo commented Jan 2, 2023

Hey @Webklex,

Thanks again for looking at the PR and I'm very happy to hear that it helped you and the project.
I will give the latest version of the library a test in the coming weeks and will happy contribute if I find something to improve.

Thanks again for making this library and I wish you all the best this new year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants