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

Add generic interfaces for classes that have no interfaces yet. #5901

Conversation

bytes-commerce
Copy link
Contributor

This change introduces Interfaces for final classes.

The reason I am proposing this change is to have the ability to Mock interfaces and without having to rely on some hacky attempts to correctly generate PHPUnit tests.

I've moved from direct declarations in constructors etc to the Interface usage. This also allows for a more dynamic development approach whilst keeping final classes at hand. They still can be decorated. However, with this, its much more strict to work with EasyAdmin as 3rd Party Developer and new contributors are less likely to introduce new classes incorrectly.

I've raised the issue here: #5890 and was redirected to the "No Final" package but its not what I am looking for; instead I look for strict code.

@bytes-commerce bytes-commerce marked this pull request as draft September 3, 2023 17:58
@OskarStark
Copy link
Collaborator

If accepted by @javiereguiluz this PR is way to big and not reviewable. We should create a meta issue and do a PR interface by interface

@bytes-commerce
Copy link
Contributor Author

@OskarStark It should have half the size; in the previous commit there is a change that I made with PHPStorm that changed nearly all files. Even tho so, its roughly 125 files changed and another 125 files added, more or less. Also the reason why I moved it to draft. Its to big to be reviewed comfortably.

@javiereguiluz
Copy link
Collaborator

I like the idea of adding some of these missing interfaces ... but I'd prefer to do it slowly. I fear that we're going to break everything by doing this 😞

So, I think we should do this:

Thanks!

@bytes-commerce
Copy link
Contributor Author

It should not break anything, but given the size and usage of this project, its much better to do it slowly.

I will create a PR just for the Interface above (as its also the one I need the most at the moment :D).

@bytes-commerce bytes-commerce deleted the interfaces-for-final-classes-and-final-declaration-for-classes branch September 5, 2023 11:24
javiereguiluz added a commit that referenced this pull request Oct 22, 2023
…arations in pr… (nopenopenope)

This PR was squashed before being merged into the 4.x branch.

Discussion
----------

Added Interface for AdminUrlGenerator and replaced declarations in pr…

…operties and constructors.

Follow up to #5901

This PR should introduce a yet not existing Interface that can be used to Mock the class in external Unit Tests.

This should act as a kind of proof of concept in order to make the EasyAdmin project more strict, yet extensible and testable.

Commits
-------

b07cea4 Added Interface for AdminUrlGenerator and replaced declarations in pr…
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.

3 participants