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

Add Body implementations #42

Merged
merged 1 commit into from
Aug 26, 2015
Merged

Add Body implementations #42

merged 1 commit into from
Aug 26, 2015

Conversation

sagikazarmark
Copy link
Member

No description provided.

@sagikazarmark sagikazarmark mentioned this pull request Aug 25, 2015
4 tasks
@hannesvdvreken
Copy link
Contributor

At Laracon EU right now. Best to not expect much from me till Thursday ;-)

@sagikazarmark
Copy link
Member Author

No problem. A gitter conversation after that?

@hannesvdvreken
Copy link
Contributor

Yup

@dbu
Copy link
Contributor

dbu commented Aug 26, 2015

i think this indeed looks like a good idea. in this case we won't have a body factory but just ask users to instantiate the appropriate object. i think for that reason it makes sense to have the common use cases right here in the contract package.

the MessageFactory should also know about the body interface i think and use it when passed a body. this does not need to be a hard dependency as instanceof can check for a non-existing class without triggering an error...

@sagikazarmark
Copy link
Member Author

i think for that reason it makes sense to have the common use cases right here in the contract package.

Well, I am struggling with that a little. But creating a php-http/body package for just a few classes which are actually part of the core doesn't seem to make much sense.

@dbu
Copy link
Contributor

dbu commented Aug 26, 2015 via email

@sagikazarmark
Copy link
Member Author

the MessageFactory should also know about the body interface

Actually the MessageFactory has nothing to do with the Client/Adapter thing. The MessageFactory package has been built for PSR-7 and I would like to make it as small as possible.

However I see the reason behind why it would make sense to handle Body classes in the message factory: some content might need custom headers. To solve this I am going to add a getContentHeaders method to the Body interface.

@sagikazarmark
Copy link
Member Author

I tried to find better names for body implementations. Opinions?

@sagikazarmark sagikazarmark self-assigned this Aug 26, 2015
Add Body implementations

Remove Data body interface, common interface is not needed

Add Content headers to Body

Body is optional, add HttpMethods trait

Refactored body implementations
@sagikazarmark
Copy link
Member Author

Merging this anyway, naming can be solved as part of Terminology.

sagikazarmark added a commit that referenced this pull request Aug 26, 2015
Add Body implementations
@sagikazarmark sagikazarmark merged commit 19453a4 into terminology Aug 26, 2015
@sagikazarmark sagikazarmark deleted the body branch August 26, 2015 20:37
use Psr\Http\Message\StreamInterface;

/**
* Allows a more input types for body data
Copy link
Contributor

Choose a reason for hiding this comment

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

grammatically confusing

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, probably mixed two different wording. "Allows special input as body"?

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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.

None yet

3 participants