Server URL Helper Cleanup #159
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Deprecate all methods and properties in the server url helper - Introduce constructor to provide the helper with a configured url - Deprecate existing tests to make them easy to remove in future Signed-off-by: George Steel <[email protected]>
Adds laminas-diactoros as a dependency so that detection of the host URI can be delegated to existing code Signed-off-by: George Steel <[email protected]>
|
Closing - This is a duplicate of #160 - I got all messed up with GHA being down |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Deprecates all methods in the server url helper and introduces a factory to provide the helper with the detected or configured host URI.
Brings in a hard dependency on laminas-diactoros in order to re-use the functions for detecting the host/port from the environment that are already present there.
Doesn't break BC according to tests, and also doesn't alter existing behaviour for long-running apps because the previous implementation caches the host/port in instance properties anyway.
The main improvement here is that consumers can configure the desired host and port without any attempt to figure it out from the environment which is particularly useful on the CLI or any other time when the required env vars are either wrong or not there at all.
The introduction of the factory is a potential BC break because an exception is thrown when there is neither configuration, nor usable env vars.