Namespace of all packages

[joelwurtz]

I would like to propose the following namespace for our different package (goal is to have, in the future, possibility to assemble all package into a single one with subsplit):

• Httplug : Http\Client (not changed)
• Promise : Http\Promise (not changed)
• Message-Factory: Http\Message (not changed)
• Tools : Http\Tools (Http\Client\Tools actually)
• Utils : Http\Utils (Http\Client\Utils actually)
• Plugin : Http\Plugin (Http\Client\Plugin actually)
• Adapters (guzzle / react / ...) : Http\Adapters\[Adapter Name], so Guzzle6 will have this namespace for example : Http\Adapters\Guzzle6 (instead of Http\Adapter actually)
• Clients (socket / curl / ...) : Http\Clients\[Client Name], so Socket will have Http\Clients\Socket
• Discovery : Http\Discovery (not changed)

WDYT ?

[sagikazarmark]

Looks good to me.

One comment: Plugin seems to be a bit inconsistent with the other namespaces, which are mostly plural (in case of a collection instead of one, separated component).

[joelwurtz]

So Http\Plugins then ?

[sagikazarmark]

Sounds logical to me 😄

[joelwurtz]

pinging client / adapters maintainers since it concerns them a lot @mekras @shulard @Nyholm @ddeboer WDYT ?

[dbu]

👍

[shulard]

Hello!

For me your proposal is a lot cleaner than the actual one! 👍

Le 19 déc. 2015 à 21:14, Joel Wurtz notifications@github.com a écrit :

pinging client / adapters maintainers since it concerns them a lot @mekras @shulard @Nyholm @ddeboer WDYT ?

—
Reply to this email directly or view it on GitHub.

[jeromegamez]

I spent hours writing this comment, so it might be that some of my thoughts got mingled. If you take something out of this, let it be two things: we have too many repos, and these are really nicely formatted tables :D.

Overview of current repos

Repo	Description	Namespace	Scope
authentication	Authenticate PSR-7 requests	Http\Authentication	Message
client-tools	Tools for HTTPlug client implementors	Http\Client\Tools	Client
cookie	Simple Cookie container logic for HTTP Requests	Http\Cookie	Message
curl-client	cURL client	Http\Curl	Client
discovery	Finds installed adapters and message factories	Http\Discovery	Adapter/Message
encoding	PSR 7 Stream for transfer / content encoding headers (chunked, deflate, compress, ...)	Http\Encoding	Message
guzzle5-adapter	Guzzle 5 HTTP adapter	Http\Adapter	Adapter
guzzle6-adapter	Guzzle 6 HTTP adapter	Http\Adapter	Adapter
helper	Helper classes for HTTP related data	Http\Helper	Message
httplug	HTTPlug, the HTTP client abstraction for PHP	Http\Client	Client
message-decorator	Decorators for PSR-7 HTTP Messages	Http\Message	Message
message-factory	Factory interfaces for PSR-7 HTTP Message	Http\Message	Message
mock-adapter	Mock HTTP Adapter for testing	Http\Adapter	Adapter
plugins	Plugin client, decorate http client with middlewares	Http\Client\Plugin	Client/Message
promise	Promise used for asynchronous HTTP requests	Http\Promise	message
psr7-vcr	Not even started yet (mea culpa :))	(none)	Client
react-adapter	(no description)	Http\Adapter	Adapter
socket-client	Socket client	Http\Socket	Client
utils	HTTP Client Utilities	Http\Client\Utils	Client/Message

Suggested changes

With this categorization in mind, I would like to suggest the following changes.

Overall: From php-http/utils, I would use just one message factory, either zend-diactoros or guzzle/psr7 - any PSR-7 compliant HTTP Client should be able to work with either of these. Both have no special dependencies, so this should just be a matter of decision.

Message-related repos

Suggestion: Merge all repos into one, e.g. php-http/message.

Reasoning: It could of course be that I want to create messages with the factory, but I don't need authentication or a special stream, but the single components don't disturb each other. At the moment, if I wanted to create an authenticated HTTP Message with Cookies, I would have to require

Repo(s)	Current Namespace(s)	New Namespace
authentication	Http\Authentication	Http\Message\Request\Authentication
cookie	Http\Cookie	Http\Message\Request\Cookie
encoding	Http\Encoding	Http\Message\Stream
helper	Http\Helper	Http\Message\Helper Http\Message\Helper\Extractor Http\Message\Helper\Normalizer Http\Message\Helper\Parser
message-decorator message-factory utils(parts)	Http\Message	Http\Message\Factory Http\Message\Factory\Decorator Http\Message\Factory\MessageFactory Http\Message\Factory\StreamFactory Http\Message\Factory\UriFactory
promise client-tools(parts)	Http\Promise	Http\Message\Promise Http\Message\Promise\FulfilledPromise Http\Message\Promise\RejectedPromise

Client-related Repos

Repo	Current Namespace	New Namespace
httplug	Http\Client	Http\Client
batch-client (extracted from utils)	N/A	Http\Client Http\Client\BatchClient (class)
client-tools	Http\Client	Http\Client\Tools
curl-client	Http\Curl	Http\Client Http\Client\CurlClient (class) (shouldn't this be an adapter?)
plugins	Http\Client\Plugin	Http\Client\Plugin
socket-client	Http\Socket	Http\Client Http\Client\SocketClient (class) (shouldn't this be an adapter?)
vcr-client/psr7-vcr	N/A	Http\Client Http\Client\VcrClient (class)

Adapter-related repos

Repo	Current Namespace	New Namespace
curl-adapter?	N/A	Http\Adapter\Curl
guzzle5-adapter	Http\Adapter	Http\Adapter\Guzzle5
guzzle6-adapter	Http\Adapter	Http\Adapter\Guzzle6
mock-adapter	Http\Adapter	Http\Adapter\Mock
react-adapter	Http\Adapter	Http\Adapter\React
socket-adapter?	N/A	Http\Adapter\Socket

[sagikazarmark]

Wow @jeromegamez I cannot find words to describe this. 😄 👍

The fact that we have too many packages came up a few times already. There are upsides and downsides of both the single repo and the multi repo solution.

but the single components don't disturb each other.

This is true, but I would at least partially modify this statement: I would exclude contracts and limit to implementations.

Contracts are probably better to keep somewhat separated so that you can require them without implementations, which is useful when you want to implement your custom stuff.

I would use just one message factory, either zend-diactoros or guzzle/psr7

I am not sure I understand this. Should we support only one? That's not really interoperable.

shouldn't this be an adapter?

Well, as you originally proposed in April when we started the whole thing, we are actually working on an HTTP Client abstraction, not an apdater one. So when we implement something new, then it is actually a client. When we implement a wrapper something already existing, then it's an adapter.

To tell the truth, the most useful proposal from your comment is the message thing for me. Message related stuff is really spread across many repositories. With the following modifications, I think that we should actually start working on php-http/message:

1. Promise is not message related. The plan is to replace it with a PSR (when it comes to reality). Although promise implementations from client-tools might be moved to the promise repository.
2. I think message factory is something that we probably want to keep separated. It should be used together with psr/http-message, so that you can provide factories with your implementation. I wasn't thinking so far that message factories should be standardized, but the more I am thinking about the more I think they should be. The fact that we have these message factories not next to the messages themselves makes me itchy.

Honestly, we really have many components which can be categorized from this point of view (used with messages) and I haven't thought about them before. Good idea @jeromegamez

[xabbuh]

I would to always use singular instead of plural (so Plugin and Adapter instead of Plugins and Adapters).Utils is the only namespace I would keep in plural.

[sagikazarmark]

Not sure if we should merge encoding into a message package as it relies on guzzle psr7.

[jeromegamez]

Sorry, I'm writing from my mobile, so please excuse my brevity :)

I don't think relying on guzzle/psr7 is a bad thing - it's a lib without dependencies and could be easily exchanged if someday something "better" comes up.

For me it's the same argument as with the message factory: we have the psr/http-message specification, and as long as the generated messages/streams are PSR-7 compliant, it shouldn't matter which lib we use to create them - each client/adapter can handle PSR-7 compliant Message-/Stream-Objects. If they are created by guzzle/psr7 or zend-diactoros or any other factory, shouldn't matter in the end. Please tell me if I'm wrong!

[sagikazarmark]

Here is an argument:

php-coveralls/php-coveralls#80 (comment)

I think it is the basic idea behind adapters, and should be for messages as well that you can use your own. So if you already use something in your project, why would you need to install another?

[jeromegamez]

Good point

[dbu]

@xabbuh can you explain why you would use singular? it feels ok to me and would require less changes, which is a plus. but are there specific reasons for it?

[xabbuh]

If the namespace were plural, I would expect each and every class from that namespace to a plugin, an Adapter and so on. While this might be true for Adapters right now a plugin usually contains lots more classes than just a single entry point class.

[ddeboer]

I would expect each and every class from that namespace to a plugin, an Adapter and so on. While this might be true for Adapters right now

Not quite: the guzzle6-adapter already contains an extra class: the wrapper (adapter?) around Guzzle promises. The fact that we (sometimes) have multiple classes per adapter makes me agree with @joelwurtz’s proposal to have a namespace per adapter, but I prefer singular too, so: Http\Adapter\Guzzle6Adapter.

Re the amount of packages: I’m not too happy with the tools and utils names; what, exactly, is the difference between the two?

[xabbuh]

Not quite: the guzzle6-adapter already contains an extra class: the wrapper (adapter?) around Guzzle promises.

Well, this then supports my reasoning for using a singular Adapter namespace even more. :)

[sagikazarmark]

Re the amount of packages: I’m not too happy with the tools and utils names; what, exactly, is the difference between the two?

Actually it is under discussion to merge the two under a better name: #99

The difference between them: Utils holds utilities for reusable library implementors, class-tools holds utilities for client implementors.

[sagikazarmark]

Okay, based on community feedback, it seems that singular namespace names are more welcome. Based on this and #99 I would like to summarize the current package reorganization plan:

authentication + message-factory-implementations + message-decorator + encoding = message
utils + client-tools = client-common

Namespaces:

Http\Client for HTTPlug and Client implementations: curl, socket, mock
Http\Adapter for adapters: Guzzle 5,6, React
Http\Message for message-factory contract and php-http/message
Http\Client\Common for common client stuff: batch client, methods client, client decorators and emulators (merged utils and client-tools)

Did I mis(understood) something?

(Not as nice as @jeromegamez's 😉 )

[jeromegamez]

I like it! 👍

[joelwurtz]

Some questions left :

• Should we have a subnamespace for each adapter / client implementation : Http\Adapter\Guzzle5 / Http\Adapter\Guzzle6 / ... ?
• Where does Plugin stand in all this namespace ?

[dbu]

how do you mean? if i got this correctly i am -1: if things depend on order of autoload or explode if for some reason i have more than one client/adapter in my project (e g partial migration) this is not nice. and its vety implicit

[xabbuh]

I think what @sagikazarmark means is that in the following code I would maybe just have to replace use Http\Adapter\Guzzle6\Client with use Http\Adapter\Buzz\Client when I want to switch the implementation being used in my project:

use Http\Adapter\Guzzle6\Client;

$client = new Client(...);

// ...

[sagikazarmark]

@xabbuh exactly.

[xabbuh]

By the way, I don't think that this is really relevant (constructor arguments can differ or people just use a DI container). However, I like Client as the name as it is short, self-explanatory and it will be consistent throughout the different implementations.

[ddeboer]

The only annoying thing with Http\Adapter\Guzzle6\Client is that Client is more common in projects than Guzzle6Client so IDE auto-completion will not work as well. Still, I prefer the simple Client for the reasons that @xabbuh mentions.

[sagikazarmark]

@xabbuh We usually try to achieve to allow using adapters with zero-config. In that case it might be useful.

@ddeboer it depends on the IDE. For example PHPStorm is smart enough to mitigate this problem.

[xabbuh]

@sagikazarmark Cool! That's even better then.

[dbu]

ah, got it. yeah Client as class name is fine. people not liking that can always alias to something else in the use statemenz.

[ddeboer]

Implemented the new namespace setup in our Guzzle6 adapter. Last thing that needs to be decided for the adapters is whether the class name should be Client or Adapter, as discussed on the PR.

[sagikazarmark]

Ideas:

• Http\Adapter\Guzzle6\Client
• Http\Adapter\Guzzle6\Adapter
• Http\Guzzle6Adapter\Client

[dbu]

the issue says namespace, but we talk about the FQN here, right? my vote goes for `Http\Guzzle6Adapter\Client` but i can live with all of them. having adapter in the namespace and client as class name seems a good idea to me.

[sagikazarmark]

the issue says namespace, but we talk about the FQN here, right?

Yeah, this discussion is kinda for everything now.

So namespaceing then (and FQCNs):

Adapters: Http\SomeAdapter\Client
Clients: Http\SomeClient\Client

[dbu]

+1

[xabbuh]

I would prefer a schema that follows Http\Adapter\<name>\Client.

[sagikazarmark]

Http\Adapter\Guzzle6\Client or Http\Adapter\Guzzle6Adapter\Client

Too long and the second is redundant. Why do you prefer this version?

[dbu]

Http\Adapter\Guzzle6\Client looks good to me too. agree that Http\Adapter\Guzzle6Adapter\Client would be redundant.

[xabbuh]

@sagikazarmark I meant the former one (the latter is indeed redundant). This way all adapters will be under a common namespace.

[ddeboer]

@sagikazarmark I’m fine with Http\Adapter\Guzzle6\Client, too. Do you have any objections to it? The only problem I see is the one mentioned here: usually external clients (such as Guzzle) have a class called Client, which may cause some confusion with our adapter’s (HTTPlug) Client class. However, I don’t really mind doing:

use GuzzleHttp\Client as GuzzleClient;
use Http\Adapter\Guzzle6\Client;

$guzzleClient = new GuzzleClient(…);
$httplugClient = new Client($guzzle);

[sagikazarmark]

Fine by me as well.

[dbu]

i would do this to be explicit:

use GuzzleHttp\Client as GuzzleClient;
use Http\Adapter\Guzzle6\Client as GuzzleAdapter;

[sagikazarmark]

I think this is resolved, isn't it?

[ddeboer]

Yes.

[joelwurtz]

I think react and curl adpater / client need to change their namespace, but maybe better to do an issue directly on the repository.

[sagikazarmark]

You are right. Issues are opened.