Open joshdifabio opened 7 years ago
Just be sure I'm understanding this approach, the following lines in a project's composer.json
"require": {
"async-interop/event-loop": "^0.4",
"async-interop/event-loop-implementation": "^0.4"
}
would become
"require": {
"async-interop/event-loop-api": "^0.4",
"async-interop/event-loop-implementation": "^0.4"
}
with loop implementations being the only libs to require async-interop/event-loop
, allowing for separate versioning of the async-interop/event-loop
as long as the API remains compatible.
If that's correct, I like it.
@trowski That's how I'd see it, yes.
Regarding packages which require event-loop-implementation
; I would suggest that those dependencies are only necessary in packages which call Loop::execute()
or Driver::run()
. Personally, I would only add that dependency to my application, not to any standalone libs I work on unless they actually run the loop. How do others see that working?
@joshdifabio I see it the other way round. event-loop-implementation
should be required by all libraries, applications don't need it, as they require a implementation that provides it anyway.
I thought about people wrongly depending on event-loop
instead of event-loop-api
, but that doesn't really matter, because if we break the interface, event-loop
will get a new major version and nothing will break for these people.
Would you mind providing a PR?
I thought about people wrongly depending on event-loop instead of event-loop-api, but that doesn't really matter, because if we break the interface, event-loop will get a new major version and nothing will break for these people.
Yeah, that's why I thought the safest thing would be for event-loop
represent the SPI + API.
Would you mind providing a PR?
Will do.
If we expect libs to always require both event-loop-api
and event-loop-implementation
, should we just amalgamate them? Perhaps we can just use event-loop-implementation
instead and loop implementors can use the current event-loop
API version as their event-loop-implementation
version? It feels wrong to have the duplication otherwise.
Edit: We could even add something like conflict: { async-interop/event-loop-implementation: "<1.0 >1.0" }
to event-loop to guarantee that implementations use the correct API version.
@joshdifabio event-loop-api
and event-loop-implementation
are two different things. event-loop-api
is the accessor and factory API / basically everything except Driver
. event-loop-implementation
on the other hand exists only to make it easy to find implementations on Packagist and to remind application developers that they actually have to require some implementation to run their app, as we use a magic bootstrap factory.
What does that conflict thing really add? How does it guarantee that?
@kelunik is there value in making the two separate? What would the version number of event-loop-implementation
mean?
What does that conflict thing really add? How does it guarantee that?
Imagine event-loop
is at v0.5 and the API is at v0.2. In this case, I think we could do the following in event-loop
:
{
"name": "async-interop/event-loop",
"conflict": {
"async-interop/event-loop-implementation": "<0.2 >0.2"
}
}
Now, if a loop implementation used the wrong API version, for example:
{
"name": "foo/loop",
"requires": { "async-interop/event-loop": "^0.5" }
"provides": { "async-interop/event-loop-implementation": "0.1" }
}
Composer would prevent this buggy loop implementation from being used because of the conflict clause in event-loop
.
Anyway, I don't think the conflict clause is that important; the important question to me is whether we want to force people to always require two packages instead of one and what the version of event-loop-implementation
would mean in that context.
In my experience, if doing one thing implicitly means doing another, the two should be grouped together.
Even if they mean different things, I think we can kill event-loop-implementation
, because if packages only depend on event-loop-api
and not event-loop
, something has to provide event-loop
anyway, that's the loop implementation then.
All libraries what depend on directly is actually the API. The API depends on an implementation being provided.
In case you access e.g. the Driver interface directly, then you should specify implementation too. (to restrict what the API depends on.) But most libraries don't.
{
"name": "async-interop/event-loop",
"requires": { "async-interop/event-loop-implementation": "0.5" }
"provides": { "async-interop/event-loop-api": "0.2" }
}
And for the loop impl:
{
"name": "foo/loop",
"requires": { "async-interop/event-loop": "^0.5" }
"provides": { "async-interop/event-loop-implementation": "0.5" }
}
and for libraries:
{
"name": "foo/library",
"requires": { "async-interop/event-loop-api": "^0.2" }
}
(and applications similar to libraries, just they require a concrete implementation too.
@bwoebi The event-loop-implementation
package is unnecessary, as it's just always the same version as event-loop
directly.
I think I like Bob's suggestion, even if the circular dependencies feel a bit dirty.
@bwoebi The event-loop-implementation package is unnecessary, as it's just always the same version as event-loop directly.
I think the loop implementations need to provide something in order to be searchable on Packagist, so event-loop-implementation
is needed for that reason?
@kelunik The event-loop requires an implementation when being used.
I think I like Bob's suggestion, even if the circular dependencies feel a bit dirty.
The fact is that there is a circular dependency. The implementation depends on the driver and the loop standard needs a Driver to work.
[To make it more cleanly we could split driver and impl, but I do not see much point in that except a little purism.]
[To make it more cleanly we could split driver and impl, but I do not see much point in that except a little purism.]
I thought about that, too. Makes PRs harder though, but there shouldn't anyway be many changes after the final version is released.
@kelunik The event-loop requires an implementation when being used.
It's enough for packages to have event-loop
/ an implementation in require-dev
. If they depend on event-loop-api
and not event-loop
, Composer will report that event-loop
is missing.
After thinking about it, I think I prefer separate packages, makes it all cleaner and avoids confusing virtual packages.
Interesting. So we'd have something like this?
some-loop-impl requires event-loop-driver, provides event-loop-driver-implementation
event-loop requires event-loop-driver, event-loop-driver-implementation
third-party-thing requires event-loop
Is that what you mean? Seems good to me!
Basically this, just without ripping this repo apart.
@bwoebi Without ripping this repo apart, we end up with having third party packages require event-loop-api
instead of event-loop
.
Splitting it is stupid, as Driver
depends on InvalidWatcherException
etc.
loop-impl requires event-loop, provides event-loop-implementation
event-loop requires event-loop-implementation, provides event-loop-api
third-party-thing requires event-loop-api
app requires loop-impl, third-party-thing
Splitting it is stupid, as Driver depends on InvalidWatcherException etc.
Good point.
I'll make a PR this evening unless someone else does it first.
@joshdifabio Ping, you wanted to provide a PR.
Apologies, this slipped my mind. I'll provide a PR this afternoon.
Do we intend to use semantic versioning for these standards? I assume that we do and that the semver guarantees will apply to both the service provider interface (SPI) and the client API, as is standard within the PHP ecosystem. If that is the case, I think we should consider also maintaining a semantic version number for only the client API. Perhaps this could be done within composer.json:
Client packages could then require async-interop/event-loop-api while loop implementations would require async-interop/event-loop.
This would allow us to make breaking changes to the SPI, for example adding a method to
Driver
, without breaking all the client packages of event-loop.