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

Upgrade to modern PHP #7

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

waxim
Copy link
Owner

@waxim waxim commented Apr 19, 2023

This is a pull request to modernize the code of this library, setting the minimm requirement at PHP8. With this PR we,

  • Upgrade to PHPUnit 10, and update the tests accordingly (new base class, deprecation of the @ExpectedException docblock.
  • Added PHPStan to the code to enable static analysis.
  • Adds strict types and return types to our functions
  • Add a new magic __call function to allow for magic getters and setters using getOurValue() or setOurValue('thing') operations to get and set data on our data array, where OurValue maps to our_value in the data array.
  • Update the README.

** Ignore failed styleci actions.
** Code outside of this PR doesn't overly need to be considered, except where traits of interfaces may have an impact.

@@ -6,5 +6,9 @@
"psr-4": {
"Vouchers\\": "src/Vouchers"
}
},
"require-dev": {
"phpunit/phpunit": "^10.0",

Choose a reason for hiding this comment

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

It would be great to add "php": 8 to ensure that composer would complain given any package in the future isn't compatible.

@@ -46,7 +46,7 @@ public function __construct(\Vouchers\Voucher\Model $model = null)
*
* @return void
*/
public function fill($number = 0)
public function fill($number = 0) :void

Choose a reason for hiding this comment

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

The spacing is it a little off here... ideally "): void"

@@ -46,7 +46,7 @@ public function __construct(\Vouchers\Voucher\Model $model = null)
*
* @return void
*/
public function fill($number = 0)
public function fill($number = 0) :void

Choose a reason for hiding this comment

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

Should we had int here as a type

*/
public function find($code)
public function find($code) :bool|Voucher

Choose a reason for hiding this comment

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

Be great to add boolean here too.

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.

None yet

2 participants