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

Refactor codebase #8

Closed
kusaasira opened this issue Oct 25, 2022 · 9 comments
Closed

Refactor codebase #8

kusaasira opened this issue Oct 25, 2022 · 9 comments

Comments

@kusaasira
Copy link
Owner

kusaasira commented Oct 25, 2022

Refactor codebase to follow DRY / SOLID and reduce coupling.

Follow PSR-12 coding standards and guidelines.

@RoadSigns
Copy link
Contributor

@kusaasira Are you able to provide a link to the Appblocks Standards and Guidelines to ensure that they are being following correctly by anyone that picks these up?

@kusaasira
Copy link
Owner Author

@RoadSigns since you added linting support for PSR-12. I'm suggesting we switch to following the PSR-12 coding standards and guidelines. Wouldn't that suffice?

@RoadSigns
Copy link
Contributor

RoadSigns commented Oct 26, 2022

@kusaasira
Yeah, that would resolve a lot of the coding standards. For the tackling of "DRY", I think we could look into using PHPStan (https://phpstan.org/) for that or just use code reviews via pull requests to allow people to capture that themselves.

@RoadSigns
Copy link
Contributor

@kusaasira By the way, there is also a script now in the package called composer cs-fix which will auto fix your coding standards to meet the PSR-12. So following the standard is already implemented and is now enforced via the tests

@kusaasira
Copy link
Owner Author

@RoadSigns the composer cs-fix is a great deal here. Adding a note that it should follow PSR-12 is a heads up though.

Concerning PHP Stan, If you are familiar, I would be grateful if you take lead here as am not that conversant though curious here.

@RoadSigns
Copy link
Contributor

@kusaasira
Sure, I can set up PHPStan in a pull request and we can discuss there around the setup.
For PHPStan, do you want it to be run via Github Actions as well?

@kusaasira
Copy link
Owner Author

@RoadSigns yes, opening a PR sounds good.

Yes, setting it up via Github Actions.

@RoadSigns
Copy link
Contributor

RoadSigns commented Oct 29, 2022

@kusaasira So turns out we were unable to achieve PHPStan above level 1.
https://phpstan.org/user-guide/rule-levels

So I've taken on the refactoring on the package so we are able to support searches across all scenarios.
Rough example of this is:

$uganda = new Uganda();
$uganda->district('Bukomansimbi')->villages();

This will give us all of the Villages inside of Bukomansimbi.
I believe this might resolve #4 and #6

From this refactor we are able to follow PSR-12 and also support up to PHPStan Level 9.
Refactor Pull Request #10

@kusaasira
Copy link
Owner Author

@RoadSigns you did some good work over there in this PR #10. The previous codebase was locked into only json encoded data. Kudos!

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

No branches or pull requests

2 participants