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

Remove references to unused EnvironmentInterface #2625

Merged
merged 1 commit into from Apr 8, 2019
Merged

Remove references to unused EnvironmentInterface #2625

merged 1 commit into from Apr 8, 2019

Conversation

jdrieghe
Copy link

@jdrieghe jdrieghe commented Apr 5, 2019

One of the proposed solutions in this issue (#2548) is to no longer reference the EnvironmentInterface in the docs, which I believe extends to type hints in the code.

I don't think removing EnvironmentInterface completely is possible without shipping a breaking change as people might have extended the Environment specifically implemented the EnvironmentInterface again in their extension.

Unless someone is willing to rework the EnvironmentInterface and the rest of the framework to accept any implementation of this interface, I think this fix might be serviceable to manage developer expectations.

If this solution is accepted I will create a similar pull request to the documentation to replace these references: https://github.com/slimphp/Slim-Documentation/search?q=EnvironmentInterface&unscoped_q=EnvironmentInterface

@coveralls
Copy link

coveralls commented Apr 5, 2019

Coverage Status

Coverage remained the same at 97.832% when pulling b1c689b on jdrieghe:remove-pointless-environment-interface into 14555a1 on slimphp:3.x.

@@ -18,7 +17,7 @@
* This is particularly useful for unit testing, but it also lets us create
* custom sub-requests.
*/
class Environment extends Collection implements EnvironmentInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

This bit feels like worth keeping to me, the class does implement the interface and who knows how it's used/hinted downstream.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. Will put it back later tonight.

@akrabat akrabat added the Slim 3 label Apr 8, 2019
@akrabat akrabat merged commit b1c689b into slimphp:3.x Apr 8, 2019
akrabat added a commit that referenced this pull request Apr 8, 2019
@akrabat
Copy link
Member

akrabat commented Apr 8, 2019

Thanks @jdrieghe

@jdrieghe
Copy link
Author

jdrieghe commented Apr 8, 2019

@akrabat
Copy link
Member

akrabat commented Apr 8, 2019

@jdrieghe jdrieghe deleted the remove-pointless-environment-interface branch April 8, 2019 11:06
@l0gicgate l0gicgate added this to the 3.13.0 milestone Apr 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants