Skip to content

Updated docs for the bundle #94

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 1 commit into from
Mar 1, 2016

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Feb 29, 2016

No description provided.

@sagikazarmark
Copy link
Member

Not sure if putting full config on a separate page is a good idea.

@Nyholm
Copy link
Member Author

Nyholm commented Mar 1, 2016

Okey, I thought the page would be too long otherwise. I move it.

I have a question about "integration with other bundles". I do not agree with what our documentation says. That will not allow a user to change client in an easy way.

I've done like this in a few projects: https://github.com/Happyr/TranslationBundle#default-configuration But Im not sure that is a good best practice either.

@sagikazarmark
Copy link
Member

Well, can be a separate page, but in a deeper level then. Other integrations might be in this section as well.

integration with other bundles? Where is it?

@Nyholm
Copy link
Member Author

Nyholm commented Mar 1, 2016

@sagikazarmark
Copy link
Member

What I did was a copy paste from the bundle README. Doesn't mean we cannot write better documentation. 😉

@Nyholm
Copy link
Member Author

Nyholm commented Mar 1, 2016

I started php-http/HttplugBundle#58 for this discussion.

@dbu
Copy link
Contributor

dbu commented Mar 1, 2016

i am +1 for having a separate configuration reference page that is not part of the documentation. the reference must list each option and its meanings. the other page should explain the common use cases. those two things are different.


Install the Httplug bundle with composer and enabled it in your AppKernel.php.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/enabled/enable

@@ -80,10 +66,11 @@ By default we use Puli to auto discover factories. If you do not want to use aut
stream_factory: Http\Message\StreamFactory\GuzzleStreamFactory



Configure your client
Copy link
Contributor

Choose a reason for hiding this comment

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

"Configure Clients"

(and all non-glue words in titel are capitalized)

@dbu
Copy link
Contributor

dbu commented Mar 1, 2016

cool. lets wrap this up asap and then merge.

@sagikazarmark
Copy link
Member

Wow, cool thing @dbu. Wanna do the fixes yourself?

@dbu
Copy link
Contributor

dbu commented Mar 1, 2016

i might find a moment tomorrow if tobias does not have the time

@Nyholm
Copy link
Member Author

Nyholm commented Mar 1, 2016

Sorry, read your comments now. Im doing some fixes now.

@Nyholm Nyholm force-pushed the symfony_bundle_2 branch from 00e8328 to 5845636 Compare March 1, 2016 16:36
@Nyholm
Copy link
Member Author

Nyholm commented Mar 1, 2016

This PR is ready to be merged into #93

sagikazarmark added a commit that referenced this pull request Mar 1, 2016
@sagikazarmark sagikazarmark merged commit eacd0af into php-http:symfony_bundle Mar 1, 2016
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