Skip to content

Add prepend and append strategy #68

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

Merged
merged 3 commits into from
Jul 3, 2016
Merged

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Jul 2, 2016

Im currently working on HTTPlug bundle and I will soon be in need for prepending a strategy to the existing ones. That is why I need a getter.

@sagikazarmark
Copy link
Member

Why do you need a getter? Wondering about valid usecases, because strategies are internal thing, we shouldn't rely on them.

@Nyholm
Copy link
Member Author

Nyholm commented Jul 2, 2016

I have an idea (work in progress). The HTTPlug's web toolbar shows all requests made by registered clients. But if a third party library uses discovery to find a client, that request will not be logged in the toolbar.

My plan in to prepend the current list of strategies with a custom one made for HTTPlug. It would be a strategy of registered clients. I think I can make that work... Anyhow I need to do something like this:

$currentStrategies = HttpClientDiscovery::getStrategies();
$strategies = array_unshift($currentStrategies, CustomStrategy::class);
HttpClientDiscovery::setStrategies(strategies);

@sagikazarmark
Copy link
Member

For that, I would rather suggest something like prependStrategy method. (You can also add appendStrategy)

@sagikazarmark sagikazarmark changed the title Added getter for $strategies Add prepend and append strategy Jul 3, 2016
@sagikazarmark
Copy link
Member

Would you prefer to merge and release 0.9.x or you wait for 1.0 in the HttplugBundle PR?

@Nyholm
Copy link
Member Author

Nyholm commented Jul 3, 2016

Let's wait a couple of days.. Ill reference this PR in a larger one in HttplugBundle

@sagikazarmark
Copy link
Member

Okay, let's wait with the tag, but is this ready? Can it be merged?

@Nyholm
Copy link
Member Author

Nyholm commented Jul 3, 2016

Yes. I believe so.

@Nyholm Nyholm merged commit 57eb7e4 into php-http:master Jul 3, 2016
@Nyholm Nyholm deleted the strategy-setter branch July 3, 2016 08:25
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.

2 participants