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 amphp/file and amphp/uri dependencies #80

Merged
merged 13 commits into from
Jan 4, 2019
Merged

Conversation

trowski
Copy link
Member

@trowski trowski commented Jan 1, 2019

Replaces async loading of hosts and resolver files with blocking reads by default. An async file loader is also provided.

HostLoader is now an interface, therefore is a BC break, though likely would not affect most users.

The third commit uses the async loader by default if amphp/file is installed. Thoughts?

Closes #78.

@trowski trowski changed the title Remove dependencies Remove amphp/file and amphp/uri dependencies Jan 1, 2019
@trowski
Copy link
Member Author

trowski commented Jan 1, 2019

Forgot about also removing amphp/uri by dropping the two functions used into this library. By including these functions in this library, we can use them in amphp/socket and deprecate amphp/uri.

@PeeHaa
Copy link

PeeHaa commented Jan 1, 2019

I will need to test these, but I have a feeling this is going to make it much more stable \o/

I am not entirely sure yet about the name here though. Maybe BlockingHostLoader to make it clear from the outside what it is / does? I will play with it a bit and see if it's more stable (at least on windows).

@trowski
Copy link
Member Author

trowski commented Jan 1, 2019

@PeeHaa I think you're right that 'blocking' is more clear, so I renamed SyncConfigFileReader to BlockingConfigFileReader. DefaultHostLoader is dependent on the ConfigFileReader instance passed, so it's not necessarily blocking.

lib/UnixConfigLoader.php Outdated Show resolved Hide resolved
lib/Internal/Socket.php Outdated Show resolved Hide resolved
@kelunik kelunik merged commit d26f9bb into master Jan 4, 2019
@kelunik kelunik deleted the remove-dependencies branch January 4, 2019 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Remove amphp/file dependency
3 participants