search

phpro / grumphp

A PHP code-quality tool
MIT License
4.11k stars 429 forks source link

Support for AMPHP v3 #1064

Closed stof closed 1 year ago

stof commented 1 year ago

Currently, grumphp requires AMPHP v2. It would be great if it could support both v2 and v3 to avoid blocking upgrades in projects using grumphp.

veewee commented 1 year ago

Idd surely accept a PR for that.

Currently, amphp/parallel (and parallel-functions) doesnt have a stable release yet - so we're kinda waiting on that.

Not sure if it is possible to support both v2 and v3 at the same time, since the whole API of amp has changed. Our TaskHandler system relies on amp v2 Promise, which has been replaced by Future in v3 for example.

So, unless there are better ideas, I guess the best thing to do here is changing the API in grumphp v2 and let it use amp 3 Futures.

veewee commented 1 year ago

Good news, an amp 3 compatible version of parallel has been released https://github.com/amphp/parallel/releases/tag/v2.0.0

stof commented 1 year ago

but parallel-functions does not have a compatible version yet

trowski commented 1 year ago

So I've had this page open on my laptop for about 2 months now, meaning to say I'd be happy to help transition this package to use amphp/parallel instead of amphp/parallel-functions as no further versions of that package is planned.

@stof You opening an issue kicked me to actually make a post. I'm not sure when I could promise any time, but I'd be happy to answer questions if someone more familiar with the library wants to tackle it first.

veewee commented 1 year ago

Thanks for jumping on on the discussion @trowski. Really love the work you (and the rest of the team) did on amp!

I'm planning to play around with amp v3 and GrumPHP for the rest of the day. This will probably result in a draft PR on which we can discuss how to do specific things.

stof commented 1 year ago

@trowski actually, opening the issue on amphp/parallel-functions was in relation to this issue :smile:

veewee commented 1 year ago

A small update: I think I covered all existing grumphp cases with amp v3 in the PR #1084.

I was able to throw away a lot of strange quirks I needed to add to get things like stop-on-failure and parallel-functions to work in v2 of amp. It's nice to see how much the project has improved! Great stuff!

Feel free to go through the PR and suggest improvements if I missed or misused anything :)

veewee commented 1 year ago

Feel free to play around with the v2.x branch; I'm planning to add some other breaking changes before releasing it. In the meantime : all feedback is welcome.